You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Lots of issues you can see from the new codebase by searching the repo for "nil dereference".
Tools exist for static analysis in Go like nilaway
However, we ended up with over 1000 errors which seemed like too many to address.
One of the issues with nilaway is that it doesn't have a way of encoding invariants in the type system.
For example:
funcIsParenthesizedTypeNode(node*Node) bool {
returnnode.Kind==SyntaxKind.ParenthesizedType
}
funcSkipTypeParentheses(node*Node) *Node {
forIsParenthesizedTypeNode(node) {
node=node.Type()
}
returnnode
}
// Acts as a helper to just grab the `Type` property// from various node types.func (n*Node) Type() *Node {
switchn.Kind {
caseSyntaxKind.ParenthesizedType:
returnn.AsParenthesizedType().Type// ...
}
In that example, nilaway complains that node may be nil in the call to node.Type(), but we know that it can't be because of the invariant established by IsParenthesizedTypeNode.
Back in TypeScript, we would just grab the property node.type because IsParenthesizedTypeNode would have been a type guard that would narrow the type of node to ParenthesizedTypeNode.
Now we have a virtual call and nilaway can't link whatever we've learned from the call to IsParenthesizedTypeNode.
So @gabritto has been prototyping a linting pass that is able to encode much of the same information from TypeScript's control flow analysis and type guards over our Go codebase.
// Type alias that is purely for documentation purposes.typeCallExpressionNode=Node//ref: struct { Kind KindCallExpression; data DefPtr[CallExpression] }// DefCallExpressionNode is a non-nilable pointer to a CallExpressionNode.typeDefCallExpressionNode=*CallExpressionNode//ref: nonnil// A type guard://ref: node is DefCallExpressionNodefuncIsCallExpression(nodeDefNode) bool {
returnnode.Kind==SyntaxKind.CallExpression
}
// A union type!typeNodeWithText=Node//ref: IdentifierNode | NumberLiteralNodetypeDefNodeWithText=*NodeWithText//ref: nonnil
Could we try to make the default non-nilable?
Would probably make a lot of stuff less-specially-annotated.
Would we be doing this checking on all types or just some subset of ours?
All right now.
Does this work well given that these are just aliases in Go?
Go is good at preserving type aliases.
How does this system know that something is a "discriminated union"?
It's similar to what we do in TypeScript.
We basically look for specific literal values in a common property across the set of types.
Basically the comment format uses Go syntax for structs along with new syntax for unions.
Strict
nilCheckingLots of issues you can see from the new codebase by searching the repo for "nil dereference".
Tools exist for static analysis in Go like nilaway
However, we ended up with over 1000 errors which seemed like too many to address.
One of the issues with nilaway is that it doesn't have a way of encoding invariants in the type system.
For example:
In that example, nilaway complains that
nodemay be nil in the call tonode.Type(), but we know that it can't be because of the invariant established byIsParenthesizedTypeNode.node.typebecauseIsParenthesizedTypeNodewould have been a type guard that would narrow the type ofnodetoParenthesizedTypeNode.IsParenthesizedTypeNode.So @gabritto has been prototyping a linting pass that is able to encode much of the same information from TypeScript's control flow analysis and type guards over our Go codebase.
[[ Example of it catching issues like Fix porting bug in isArgumentAndStartLineOverlapsExpressionBeingCalled typescript-go#1948 ]]
Uses comment suffixes.
Could we try to make the default non-nilable?
Does this work well given that these are just aliases in Go?
How does this system know that something is a "discriminated union"?
Basically the comment format uses Go syntax for structs along with new syntax for unions.
Looks extremely promising so far.