-
Notifications
You must be signed in to change notification settings - Fork 603
feat: move nil check to call sites #658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@rbuckton Can you take a look at this? |
@SoulPancake Sorry for jumping on here for a slightly tangential issue, but I noticed that nil checks are inconsistent throughout the codebase. For example, https://github.com/microsoft/typescript-go/blob/main/internal%2Fast%2Fast.go#L15-L31 type Visitor func(*Node) bool
func visit(v Visitor, node *Node) bool {
if node != nil {
return v(node)
}
return false
}
func visitNodes(v Visitor, nodes []*Node) bool {
for _, node := range nodes {
if v(node) {
return true
}
}
return false
}
|
@SaadiSave Agreed, I will add the nil check in the loop |
@SoulPancake is there a lint to enforce nil checks? |
This addresses
// TODO(rbuckton): Move
node != niltest to call sites
@rbuckton Can you please review this
I can undo the stylistic whitespace changes in the internal/ast/utilities.gbut I think they're for the better, LMK what you think