Skip to content

Commit

Permalink
Remove redundant nil check (#1367)
Browse files Browse the repository at this point in the history
From the Go docs [1]:

  "1. For a nil slice, the number of iterations is 0."
  "3. If the map is nil, the number of iterations is 0."

Therefore, an additional nil check for before the loop is unnecessary.

[1]: https://go.dev/ref/spec#For_range

Signed-off-by: Eng Zer Jun <[email protected]>
  • Loading branch information
Juneezee committed Aug 23, 2023
1 parent 9ad9e21 commit 12f3b14
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 109 deletions.
16 changes: 6 additions & 10 deletions internal/pkg/cli/flag_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,9 @@ func (flag *Flag) Owns(input string) bool {
if flag.name == input {
return true
}
if flag.altNames != nil {
for _, name := range flag.altNames {
if name == input {
return true
}
for _, name := range flag.altNames {
if name == input {
return true
}
}
return false
Expand All @@ -414,11 +412,9 @@ func (flag *Flag) Matches(input string) bool {
if strings.Contains(flag.name, input) {
return true
}
if flag.altNames != nil {
for _, name := range flag.altNames {
if strings.Contains(name, input) {
return true
}
for _, name := range flag.altNames {
if strings.Contains(name, input) {
return true
}
}
return false
Expand Down
6 changes: 2 additions & 4 deletions internal/pkg/dsl/ast_print.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,8 @@ func (node *ASTNode) printAux(depth int) {
fmt.Println()

// Children, indented one level further
if node.Children != nil {
for _, child := range node.Children {
child.printAux(depth + 1)
}
for _, child := range node.Children {
child.printAux(depth + 1)
}
}

Expand Down
10 changes: 4 additions & 6 deletions internal/pkg/dsl/cst/for.go
Original file line number Diff line number Diff line change
Expand Up @@ -887,12 +887,10 @@ func (node *TripleForLoopNode) Execute(state *runtime.State) (*BlockExitPayload,
}

for {
if node.precontinuationAssignments != nil {
for _, precontinuationAssignment := range node.precontinuationAssignments {
_, err := precontinuationAssignment.Execute(state)
if err != nil {
return nil, err
}
for _, precontinuationAssignment := range node.precontinuationAssignments {
_, err := precontinuationAssignment.Execute(state)
if err != nil {
return nil, err
}
}
if node.continuationExpressionNode != nil { // empty is true
Expand Down
68 changes: 32 additions & 36 deletions internal/pkg/dsl/cst/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,21 @@ func ValidateAST(
}
}

if ast.RootNode.Children != nil {
for _, astChild := range ast.RootNode.Children {
err := validateASTAux(
astChild,
dslInstanceType,
atTopLevel,
inLoop,
inBeginOrEnd,
inUDF,
inUDS,
isMainBlockLastStatement,
isAssignmentLHS,
isUnset,
)
if err != nil {
return err
}
for _, astChild := range ast.RootNode.Children {
err := validateASTAux(
astChild,
dslInstanceType,
atTopLevel,
inLoop,
inBeginOrEnd,
inUDF,
inUDS,
isMainBlockLastStatement,
isAssignmentLHS,
isUnset,
)
if err != nil {
return err
}
}

Expand Down Expand Up @@ -219,25 +217,23 @@ func validateASTAux(
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Treewalk

if astNode.Children != nil {
for i, astChild := range astNode.Children {
nextLevelIsAssignmentLHS = astNode.Type == dsl.NodeTypeAssignment && i == 0
nextLevelIsUnset = astNode.Type == dsl.NodeTypeUnset
err := validateASTAux(
astChild,
dslInstanceType,
nextLevelAtTopLevel,
nextLevelInLoop,
nextLevelInBeginOrEnd,
nextLevelInUDF,
nextLevelInUDS,
isMainBlockLastStatement,
nextLevelIsAssignmentLHS,
nextLevelIsUnset,
)
if err != nil {
return err
}
for i, astChild := range astNode.Children {
nextLevelIsAssignmentLHS = astNode.Type == dsl.NodeTypeAssignment && i == 0
nextLevelIsUnset = astNode.Type == dsl.NodeTypeUnset
err := validateASTAux(
astChild,
dslInstanceType,
nextLevelAtTopLevel,
nextLevelInLoop,
nextLevelInBeginOrEnd,
nextLevelInUDF,
nextLevelInUDS,
isMainBlockLastStatement,
nextLevelIsAssignmentLHS,
nextLevelIsUnset,
)
if err != nil {
return err
}
}

Expand Down
102 changes: 49 additions & 53 deletions internal/pkg/dsl/cst/warn.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,14 @@ func WarnOnAST(
inAssignment := false
ok := true

if ast.RootNode.Children != nil {
for _, astChild := range ast.RootNode.Children {
ok1 := warnOnASTAux(
astChild,
variableNamesWrittenTo,
inAssignment,
)
// Don't end early on first warning; tree-walk to list them all.
ok = ok1 && ok
}
for _, astChild := range ast.RootNode.Children {
ok1 := warnOnASTAux(
astChild,
variableNamesWrittenTo,
inAssignment,
)
// Don't end early on first warning; tree-walk to list them all.
ok = ok1 && ok
}

return ok
Expand Down Expand Up @@ -134,52 +132,50 @@ func warnOnASTAux(

// Treewalk to check the rest of the AST below this node.

if astNode.Children != nil {
for i, astChild := range astNode.Children {
childInAssignment := inAssignment

if astNode.Type == dsl.NodeTypeAssignment && i == 0 {
// LHS of assignment statements
childInAssignment = true
} else if astNode.Type == dsl.NodeTypeForLoopOneVariable && i == 0 {
// The 'k' in 'for (k in $*)'
childInAssignment = true
} else if astNode.Type == dsl.NodeTypeForLoopTwoVariable && (i == 0 || i == 1) {
// The 'k' and 'v' in 'for (k,v in $*)'
childInAssignment = true
} else if astNode.Type == dsl.NodeTypeForLoopMultivariable && (i == 0 || i == 1) {
// The 'k1', 'k2', and 'v' in 'for ((k1,k2),v in $*)'
childInAssignment = true
} else if astNode.Type == dsl.NodeTypeParameterList {
for i, astChild := range astNode.Children {
childInAssignment := inAssignment

if astNode.Type == dsl.NodeTypeAssignment && i == 0 {
// LHS of assignment statements
childInAssignment = true
} else if astNode.Type == dsl.NodeTypeForLoopOneVariable && i == 0 {
// The 'k' in 'for (k in $*)'
childInAssignment = true
} else if astNode.Type == dsl.NodeTypeForLoopTwoVariable && (i == 0 || i == 1) {
// The 'k' and 'v' in 'for (k,v in $*)'
childInAssignment = true
} else if astNode.Type == dsl.NodeTypeForLoopMultivariable && (i == 0 || i == 1) {
// The 'k1', 'k2', and 'v' in 'for ((k1,k2),v in $*)'
childInAssignment = true
} else if astNode.Type == dsl.NodeTypeParameterList {
childInAssignment = true
} else if inAssignment && astNode.Type == dsl.NodeTypeArrayOrMapIndexAccess {
// In 'z[i] = 1', the 'i' is a read and the 'z' is a write.
//
// mlr --from r put -v -W 'z[i] = 1'
// DSL EXPRESSION:
// z[i]=1
//
// AST:
// * statement block
// * assignment "="
// * array or map index access "[]"
// * local variable "z"
// * local variable "i"
// * int literal "1"
if i == 0 {
childInAssignment = true
} else if inAssignment && astNode.Type == dsl.NodeTypeArrayOrMapIndexAccess {
// In 'z[i] = 1', the 'i' is a read and the 'z' is a write.
//
// mlr --from r put -v -W 'z[i] = 1'
// DSL EXPRESSION:
// z[i]=1
//
// AST:
// * statement block
// * assignment "="
// * array or map index access "[]"
// * local variable "z"
// * local variable "i"
// * int literal "1"
if i == 0 {
childInAssignment = true
} else {
childInAssignment = false
}
} else {
childInAssignment = false
}
ok1 := warnOnASTAux(
astChild,
variableNamesWrittenTo,
childInAssignment,
)
// Don't end early on first error; tree-walk to list them all.
ok = ok1 && ok
}
ok1 := warnOnASTAux(
astChild,
variableNamesWrittenTo,
childInAssignment,
)
// Don't end early on first error; tree-walk to list them all.
ok = ok1 && ok
}

return ok
Expand Down

0 comments on commit 12f3b14

Please sign in to comment.