Source file src/github.com/bazelbuild/buildtools/warn/warn_control_flow.go

Documentation: github.com/bazelbuild/buildtools/warn

     1  /*
     2  Copyright 2020 Google LLC
     3  
     4  Licensed under the Apache License, Version 2.0 (the "License");
     5  you may not use this file except in compliance with the License.
     6  You may obtain a copy of the License at
     7  
     8      https://www.apache.org/licenses/LICENSE-2.0
     9  
    10  Unless required by applicable law or agreed to in writing, software
    11  distributed under the License is distributed on an "AS IS" BASIS,
    12  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    13  See the License for the specific language governing permissions and
    14  limitations under the License.
    15  */
    16  
    17  // Warnings related to the control flow
    18  
    19  package warn
    20  
    21  import (
    22  	"fmt"
    23  	"strings"
    24  
    25  	"github.com/bazelbuild/buildtools/build"
    26  	"github.com/bazelbuild/buildtools/bzlenv"
    27  	"github.com/bazelbuild/buildtools/edit"
    28  )
    29  
    30  // findReturnsWithoutValue searches for return statements without a value, calls `callback` on
    31  // them and returns whether the current list of statements terminates (either by a return or fail()
    32  // statements on the current level in all subbranches.
    33  func findReturnsWithoutValue(stmts []build.Expr, callback func(*build.ReturnStmt)) bool {
    34  	if len(stmts) == 0 {
    35  		// May occur in empty else-clauses
    36  		return false
    37  	}
    38  	terminated := false
    39  	for _, stmt := range stmts {
    40  		switch stmt := stmt.(type) {
    41  		case *build.ReturnStmt:
    42  			if stmt.Result == nil {
    43  				callback(stmt)
    44  			}
    45  			terminated = true
    46  		case *build.CallExpr:
    47  			ident, ok := stmt.X.(*build.Ident)
    48  			if ok && ident.Name == "fail" {
    49  				terminated = true
    50  			}
    51  		case *build.ForStmt:
    52  			// Call recursively to find all return statements without a value there.
    53  			// Even if a for-loop is guaranteed to terminate in each iteration, buildifier still can't
    54  			// check whether the loop is not empty, so we can't say that the statement after the ForStmt
    55  			// is unreachable.
    56  			findReturnsWithoutValue(stmt.Body, callback)
    57  		case *build.IfStmt:
    58  			// Save to separate values to avoid short circuit evaluation
    59  			term1 := findReturnsWithoutValue(stmt.True, callback)
    60  			term2 := findReturnsWithoutValue(stmt.False, callback)
    61  			if term1 && term2 {
    62  				terminated = true
    63  			}
    64  		}
    65  	}
    66  	return terminated
    67  }
    68  
    69  // missingReturnValueWarning warns if a function returns both explicit and implicit values.
    70  func missingReturnValueWarning(f *build.File) []*LinterFinding {
    71  	findings := []*LinterFinding{}
    72  
    73  	// Collect all def statements in the file
    74  	defStmts := []*build.DefStmt{}
    75  	build.WalkStatements(f, func(expr build.Expr, stack []build.Expr) (err error) {
    76  		if def, ok := expr.(*build.DefStmt); ok {
    77  			defStmts = append(defStmts, def)
    78  		}
    79  		return
    80  	})
    81  
    82  	for _, function := range defStmts {
    83  		var hasNonEmptyReturns bool
    84  		build.WalkStatements(function, func(expr build.Expr, stack []build.Expr) (err error) {
    85  			if _, ok := expr.(*build.DefStmt); ok {
    86  				if len(stack) > 0 {
    87  					return &build.StopTraversalError{}
    88  				}
    89  			}
    90  
    91  			if ret, ok := expr.(*build.ReturnStmt); ok && ret.Result != nil {
    92  				hasNonEmptyReturns = true
    93  			}
    94  			return err
    95  		})
    96  
    97  		if !hasNonEmptyReturns {
    98  			continue
    99  		}
   100  		explicitReturn := findReturnsWithoutValue(function.Body, func(ret *build.ReturnStmt) {
   101  			findings = append(findings,
   102  				makeLinterFinding(ret, fmt.Sprintf("Some but not all execution paths of %q return a value.", function.Name)))
   103  		})
   104  		if !explicitReturn {
   105  			findings = append(findings,
   106  				makeLinterFinding(function, fmt.Sprintf(`Some but not all execution paths of %q return a value.
   107  The function may terminate by an implicit return in the end.`, function.Name)))
   108  		}
   109  	}
   110  	return findings
   111  }
   112  
   113  // findUnreachableStatements searches for unreachable statements (i.e. statements that immediately
   114  // follow `return`, `break`, `continue`, and `fail()` statements and calls `callback` on them.
   115  // If there are several consequent unreachable statements, it only reports the first of them.
   116  // Returns whether the execution is terminated explicitly.
   117  func findUnreachableStatements(stmts []build.Expr, callback func(build.Expr)) bool {
   118  	unreachable := false
   119  	for _, stmt := range stmts {
   120  		if unreachable {
   121  			callback(stmt)
   122  			return true
   123  		}
   124  		switch stmt := stmt.(type) {
   125  		case *build.ReturnStmt:
   126  			unreachable = true
   127  		case *build.CallExpr:
   128  			ident, ok := stmt.X.(*build.Ident)
   129  			if ok && ident.Name == "fail" {
   130  				unreachable = true
   131  			}
   132  		case *build.BranchStmt:
   133  			if stmt.Token != "pass" {
   134  				// either break or continue
   135  				unreachable = true
   136  			}
   137  		case *build.ForStmt:
   138  			findUnreachableStatements(stmt.Body, callback)
   139  		case *build.IfStmt:
   140  			// Save to separate values to avoid short circuit evaluation
   141  			term1 := findUnreachableStatements(stmt.True, callback)
   142  			term2 := findUnreachableStatements(stmt.False, callback)
   143  			if term1 && term2 {
   144  				unreachable = true
   145  			}
   146  		}
   147  	}
   148  	return unreachable
   149  }
   150  
   151  func unreachableStatementWarning(f *build.File) []*LinterFinding {
   152  	findings := []*LinterFinding{}
   153  
   154  	build.WalkStatements(f, func(expr build.Expr, stack []build.Expr) (err error) {
   155  		def, ok := expr.(*build.DefStmt)
   156  		if !ok {
   157  			return
   158  		}
   159  		findUnreachableStatements(def.Body, func(expr build.Expr) {
   160  			findings = append(findings,
   161  				makeLinterFinding(expr, `The statement is unreachable.`))
   162  		})
   163  		return
   164  	})
   165  	return findings
   166  }
   167  
   168  func noEffectStatementsCheck(body []build.Expr, isTopLevel, isFunc bool, findings []*LinterFinding) []*LinterFinding {
   169  	seenNonComment := false
   170  	for _, stmt := range body {
   171  		if stmt == nil {
   172  			continue
   173  		}
   174  
   175  		_, isString := stmt.(*build.StringExpr)
   176  		if isString {
   177  			if !seenNonComment && (isTopLevel || isFunc) {
   178  				// It's a docstring.
   179  				seenNonComment = true
   180  				continue
   181  			}
   182  		}
   183  		if _, ok := stmt.(*build.CommentBlock); !ok {
   184  			seenNonComment = true
   185  		}
   186  		switch s := (stmt).(type) {
   187  		case *build.DefStmt, *build.ForStmt, *build.IfStmt, *build.LoadStmt, *build.ReturnStmt,
   188  			*build.CallExpr, *build.CommentBlock, *build.BranchStmt, *build.AssignExpr:
   189  			continue
   190  		case *build.Comprehension:
   191  			if !isTopLevel || s.Curly {
   192  				// List comprehensions are allowed on top-level.
   193  				findings = append(findings,
   194  					makeLinterFinding(stmt, "Expression result is not used. Use a for-loop instead of a list comprehension."))
   195  			}
   196  			continue
   197  		}
   198  
   199  		msg := "Expression result is not used."
   200  		if isString {
   201  			msg += " Docstrings should be the first statements of a file or a function (they may follow comment lines)."
   202  		}
   203  		findings = append(findings, makeLinterFinding(stmt, msg))
   204  	}
   205  	return findings
   206  }
   207  
   208  func noEffectWarning(f *build.File) []*LinterFinding {
   209  	findings := []*LinterFinding{}
   210  	findings = noEffectStatementsCheck(f.Stmt, true, false, findings)
   211  	build.WalkStatements(f, func(expr build.Expr, stack []build.Expr) (err error) {
   212  		// Docstrings are valid statements without effects. To detect them we need to
   213  		// analyze blocks of statements rather than single statements.
   214  		switch expr := expr.(type) {
   215  		case *build.ForStmt:
   216  			findings = noEffectStatementsCheck(expr.Body, false, false, findings)
   217  		case *build.DefStmt:
   218  			findings = noEffectStatementsCheck(expr.Function.Body, false, true, findings)
   219  		case *build.IfStmt:
   220  			findings = noEffectStatementsCheck(expr.True, false, false, findings)
   221  			findings = noEffectStatementsCheck(expr.False, false, false, findings)
   222  		}
   223  		return
   224  	})
   225  	return findings
   226  }
   227  
   228  // extractIdentsFromStmt returns all idents from an AST node representing a
   229  // single statement that are either defined outside the node and used inside,
   230  // or defined inside the node and can be used outside.
   231  // Examples of idents that don't fall into either of the categories:
   232  //   - Named arguments of function calls: `foo` in `f(foo = "bar")`
   233  //   - Iterators of comprehension nodes and its usages: `x` in `[f(x) for x in y]`
   234  //   - Lambda arguments: `x` in `lambda x: x + 1`
   235  //
   236  // Statements that contain other statements (for-loops, if-else blocks) are not
   237  // traversed inside.
   238  func extractIdentsFromStmt(stmt build.Expr) (assigned, used map[*build.Ident]bool) {
   239  	// The values for `assigned` are `true` if the warning for the variable should
   240  	// be suppressed, and `false` otherwise.
   241  	// It's still important to know that the variable has been assigned in the
   242  	// current scope because it could shadow a variable with the same name from an
   243  	// outer scope.
   244  	assigned = make(map[*build.Ident]bool)
   245  	used = make(map[*build.Ident]bool)
   246  
   247  	// Local scopes for comprehensions
   248  	scopes := make(map[build.Expr]map[string]bool)
   249  
   250  	// Nodes that are excluded from traversal
   251  	blockedNodes := make(map[build.Expr]bool)
   252  
   253  	build.WalkInterruptable(stmt, func(node build.Expr, stack []build.Expr) (err error) {
   254  		// Check if the current node has been blocked
   255  		if _, ok := blockedNodes[node]; ok {
   256  			return &build.StopTraversalError{}
   257  		}
   258  
   259  		switch expr := node.(type) {
   260  		case *build.AssignExpr:
   261  			// If it's a top-level assign expression, extract LValues from LHS
   262  			// Otherwise just ignore the LHS, it must be a keyword argument of a
   263  			// function call.
   264  			if node != stmt {
   265  				// The assignment expression is not the statement itself but its child,
   266  				// that means it's a keyword argument for a function call. Its LHS
   267  				// should be ignored.
   268  				blockedNodes[expr.LHS] = true
   269  				return
   270  			}
   271  
   272  			hasUnusedComment := edit.ContainsComments(expr, "@unused")
   273  
   274  			// LHS may contain both variables that are being used and variables that
   275  			// are being assigned to, e.g. in the following example:
   276  			//     x[i][f(name='foo')], y = 1, 2
   277  			// `x`, `i`, `f` are used, `y` is assigned, `name` should be ignored.
   278  			// Further traversal will ignore `name` but won't know that it's in an LHS
   279  			// of an assign expression, so it'll erroneously collect `y` as used.
   280  			// After the traversal it'll need to be removed from `used`.
   281  
   282  			// If some (but not all) variables assigned to by the statements are
   283  			// prefixed with an underscore, suppress the warning on them (i.e. allow
   284  			// them to be unused). That's a common use case for partial unpacking of
   285  			// tuples:
   286  			//
   287  			//     foo, _bar = my_function()  # only `foo` is needed
   288  			//
   289  			// However if all variables are underscored and unused, they should still
   290  			// be reported:
   291  			//
   292  			//     _foo, _bar = my_function()  # LHS can just be removed
   293  
   294  			lValues := bzlenv.CollectLValues(expr.LHS)
   295  			allLValuesUnderscored := true
   296  			for _, lValue := range lValues {
   297  				if !strings.HasPrefix(lValue.Name, "_") {
   298  					allLValuesUnderscored = false
   299  					break
   300  				}
   301  			}
   302  
   303  			for _, lValue := range bzlenv.CollectLValues(expr.LHS) {
   304  				assigned[lValue] = hasUnusedComment ||
   305  					(!allLValuesUnderscored && strings.HasPrefix(lValue.Name, "_"))
   306  			}
   307  
   308  		case *build.ForStmt:
   309  			// Like AssignExpr, ForStmt too has an analogue of LHS and RHS.
   310  			// Unlike AssignExpr, in this function they may appear only in the root of
   311  			// traversal and shouldn't be traversed inside (the caller of
   312  			// `extractIdentsFromStmt` should be responsible for checking all
   313  			// statements including those that are inside for-loops.
   314  
   315  			// It's common to not use all variables (or even not use any of them)
   316  			// after unpacking tuples, suppress the warning an all of them that are
   317  			// prefixed with an underscore:
   318  			//
   319  			//     for _, (_b, c) in iterable:
   320  			//         print(c)
   321  			for _, lValue := range bzlenv.CollectLValues(expr.Vars) {
   322  				assigned[lValue] = strings.HasPrefix(lValue.Name, "_")
   323  			}
   324  
   325  			// Don't traverse inside the inner statements (but still traverse into
   326  			// the expressions in `Vars` an `X`).
   327  			for _, substmt := range expr.Body {
   328  				blockedNodes[substmt] = true
   329  			}
   330  
   331  		case *build.IfStmt:
   332  			// Nothing special, just don't traverse the inner statements (like with
   333  			// ForStmt nodes).
   334  			for _, substmt := range expr.True {
   335  				blockedNodes[substmt] = true
   336  			}
   337  			for _, substmt := range expr.False {
   338  				blockedNodes[substmt] = true
   339  			}
   340  
   341  		case *build.Comprehension:
   342  			// Comprehensions introduce their own visibility scope that shadows the
   343  			// outer scope. Iterators that are defined and used there don't affect
   344  			// the usage of variables with the same name outside the comprehension
   345  			// scope.
   346  			scope := make(map[string]bool)
   347  			for _, clause := range expr.Clauses {
   348  				forClause, ok := clause.(*build.ForClause)
   349  				if !ok {
   350  					// if-clause
   351  					continue
   352  				}
   353  				for _, lValue := range bzlenv.CollectLValues(forClause.Vars) {
   354  					scope[lValue.Name] = true
   355  				}
   356  			}
   357  			scopes[expr] = scope
   358  
   359  		case *build.LambdaExpr:
   360  			// Similar to Comprehension nodes
   361  			scope := make(map[string]bool)
   362  			for _, param := range expr.Params {
   363  				for _, lValue := range bzlenv.CollectLValues(param) {
   364  					scope[lValue.Name] = true
   365  				}
   366  			}
   367  			scopes[expr] = scope
   368  
   369  		case *build.Ident:
   370  			// If the identifier is defined in an intermediate scope, ignore it.
   371  			for _, node := range stack {
   372  				if scope, ok := scopes[node]; ok {
   373  					if _, ok := scope[expr.Name]; ok {
   374  						return
   375  					}
   376  				}
   377  			}
   378  			used[expr] = true
   379  
   380  		default:
   381  			// Do nothing, just traverse further
   382  		}
   383  		return
   384  	})
   385  
   386  	for ident := range assigned {
   387  		// If the same ident (not the same variable but the same AST node) is
   388  		// registered as both "assigned" and "used", it means it was in fact just
   389  		// assigned, remove it from "used".
   390  		delete(used, ident)
   391  	}
   392  	return assigned, used
   393  }
   394  
   395  // unusedVariableCheck checks for unused variables inside a given node `stmt` (either *build.File or
   396  // *build.DefStmt) and variables that are used in the current scope or subscopes,
   397  // but not defined here.
   398  func unusedVariableCheck(f *build.File, root build.Expr) (map[string]bool, []*LinterFinding) {
   399  	findings := []*LinterFinding{}
   400  
   401  	// Symbols that are defined in the current scope
   402  	definedSymbols := make(map[string]*build.Ident)
   403  
   404  	// Functions that are defined in the current scope
   405  	definedFunctions := make(map[string]*build.DefStmt)
   406  
   407  	// Symbols that are used in the current and inner scopes
   408  	usedSymbols := make(map[string]bool)
   409  
   410  	// Symbols for which the warning should be suppressed
   411  	suppressedWarnings := make(map[string]bool)
   412  
   413  	build.WalkStatements(root, func(expr build.Expr, stack []build.Expr) (err error) {
   414  		switch expr := expr.(type) {
   415  		case *build.File:
   416  			// File nodes don't have anything meaningful, just traverse its subnodes.
   417  
   418  		case *build.DefStmt:
   419  			if len(stack) > 0 {
   420  				// Nested def statement. Don't traverse inside, instead call
   421  				// unusedVariableCheck recursively to handle nested scopes properly.
   422  
   423  				// The function name is defined in the current scope
   424  				if _, ok := definedFunctions[expr.Name]; !ok {
   425  					definedFunctions[expr.Name] = expr
   426  				}
   427  				if edit.ContainsComments(expr, "@unused") {
   428  					suppressedWarnings[expr.Name] = true
   429  				}
   430  
   431  				usedSymbolsInFunction, findingsInFunction := unusedVariableCheck(f, expr)
   432  				findings = append(findings, findingsInFunction...)
   433  				for symbol := range usedSymbolsInFunction {
   434  					usedSymbols[symbol] = true
   435  				}
   436  				return &build.StopTraversalError{}
   437  			}
   438  
   439  			// The function is a root for the current scope.
   440  			// Collect its parameters as defined in the current scope.
   441  			for _, param := range expr.Params {
   442  				// Function parameters are defined in the current scope.
   443  				if ident, _ := build.GetParamIdent(param); ident != nil {
   444  					definedSymbols[ident.Name] = ident
   445  					if ident.Name == "name" || strings.HasPrefix(ident.Name, "_") || edit.ContainsComments(param, "@unused") {
   446  						// Don't warn about function arguments if they start with "_"
   447  						// or explicitly marked with @unused.
   448  						// Also don't warn about unused "name" arguments, it could be a
   449  						// macro where such argument is encouraged (by `unnamed-macro`)
   450  						// even if not used.
   451  						suppressedWarnings[ident.Name] = true
   452  					}
   453  				}
   454  				// The default variables for the parameters are defined in the outer
   455  				// scope but used here.
   456  				assign, ok := param.(*build.AssignExpr)
   457  				if !ok {
   458  					continue
   459  				}
   460  
   461  				// RHS is not a statement, but similar traversal rules should be applied
   462  				// to it. E.g. it may have a comprehension node with its inner scope or
   463  				// a function call with a keyword parameter.
   464  				_, used := extractIdentsFromStmt(assign.RHS)
   465  				for ident := range used {
   466  					usedSymbols[ident.Name] = true
   467  				}
   468  			}
   469  
   470  		case *build.LoadStmt:
   471  			// LoadStmt nodes store the loaded symbols as idents, even though in the
   472  			// source code they are strings. These idents may confuse the check,
   473  			// they also shouldn't affect the warnings at all, unused loads are taken
   474  			// care of by another check. It's safe to just ignore them here.
   475  			return
   476  
   477  		default:
   478  			assigned, used := extractIdentsFromStmt(expr)
   479  
   480  			for symbol := range used {
   481  				usedSymbols[symbol.Name] = true
   482  			}
   483  			for symbol, isSuppressed := range assigned {
   484  				if _, ok := definedSymbols[symbol.Name]; !ok {
   485  					definedSymbols[symbol.Name] = symbol
   486  					if isSuppressed {
   487  						suppressedWarnings[symbol.Name] = true
   488  					}
   489  				}
   490  			}
   491  		}
   492  		return
   493  	})
   494  
   495  	// If a variable is defined in an outer scope but also in this scope, it
   496  	// shadows the outer variable. If it's used in the current scope, it doesn't
   497  	// make the variable with the same name from an outer scope also used.
   498  	// Collect variables that are used in the current or inner scopes but are not
   499  	// defined in the current scope.
   500  	usedSymbolsFromOuterScope := make(map[string]bool)
   501  	for symbol := range usedSymbols {
   502  		if _, ok := definedSymbols[symbol]; ok {
   503  			continue
   504  		}
   505  		if _, ok := definedFunctions[symbol]; ok {
   506  			continue
   507  		}
   508  		usedSymbolsFromOuterScope[symbol] = true
   509  	}
   510  
   511  	// Top-level variables defined in .bzl or generic Starlark files
   512  	// can be imported from elsewhere, even if not used in the current file.
   513  	// Do not warn on exportable variables.
   514  	ignoreTopLevel := (f.Type == build.TypeBzl || f.Type == build.TypeDefault) && root == f
   515  
   516  	for name, ident := range definedSymbols {
   517  		if _, ok := usedSymbols[name]; ok {
   518  			// The variable is used either in this scope or in a nested scope
   519  			continue
   520  		}
   521  		if _, ok := suppressedWarnings[name]; ok {
   522  			// The variable is explicitly marked with @unused, ignore
   523  			continue
   524  		}
   525  		if ignoreTopLevel && !strings.HasPrefix(name, "_") {
   526  			continue
   527  		}
   528  		findings = append(findings,
   529  			makeLinterFinding(ident, fmt.Sprintf(`Variable %q is unused. Please remove it.`, ident.Name)))
   530  	}
   531  
   532  	for name, def := range definedFunctions {
   533  		if _, ok := usedSymbols[name]; ok {
   534  			// The function is used either in this scope or in a nested scope
   535  			continue
   536  		}
   537  		if ignoreTopLevel && !strings.HasPrefix(name, "_") {
   538  			continue
   539  		}
   540  		if _, ok := suppressedWarnings[name]; ok {
   541  			// The function is explicitly marked with @unused, ignore
   542  			continue
   543  		}
   544  		findings = append(findings,
   545  			makeLinterFinding(def, fmt.Sprintf(`Function %q is unused. Please remove it.`, def.Name)))
   546  	}
   547  
   548  	return usedSymbolsFromOuterScope, findings
   549  }
   550  
   551  func unusedVariableWarning(f *build.File) []*LinterFinding {
   552  	_, findings := unusedVariableCheck(f, f)
   553  	return findings
   554  }
   555  
   556  func redefinedVariableWarning(f *build.File) []*LinterFinding {
   557  	findings := []*LinterFinding{}
   558  	definedSymbols := make(map[string]bool)
   559  
   560  	types := DetectTypes(f)
   561  	for _, s := range f.Stmt {
   562  		// look for all assignments in the scope
   563  		as, ok := s.(*build.AssignExpr)
   564  		if !ok {
   565  			continue
   566  		}
   567  		left, ok := as.LHS.(*build.Ident)
   568  		if !ok {
   569  			continue
   570  		}
   571  		if !definedSymbols[left.Name] {
   572  			definedSymbols[left.Name] = true
   573  			continue
   574  		}
   575  
   576  		if as.Op == "+=" && (types[as.LHS] == List || types[as.RHS] == List) {
   577  			// Not a reassignment, just appending to a list
   578  			continue
   579  		}
   580  
   581  		findings = append(findings,
   582  			makeLinterFinding(as.LHS, fmt.Sprintf(`Variable %q has already been defined. 
   583  Redefining a global value is discouraged and will be forbidden in the future.
   584  Consider using a new variable instead.`, left.Name)))
   585  	}
   586  	return findings
   587  }
   588  
   589  func unusedLoadWarning(f *build.File) []*LinterFinding {
   590  	findings := []*LinterFinding{}
   591  	loaded := make(map[string]struct {
   592  		label, from string
   593  		line        int
   594  	})
   595  
   596  	symbols := edit.UsedSymbols(f)
   597  	types := edit.UsedTypes(f)
   598  	for stmtIndex := 0; stmtIndex < len(f.Stmt); stmtIndex++ {
   599  		originalLoad, ok := f.Stmt[stmtIndex].(*build.LoadStmt)
   600  		if !ok {
   601  			continue
   602  		}
   603  
   604  		// Findings related to the current load statement
   605  		loadFindings := []*LinterFinding{}
   606  
   607  		// Copy the `load` object to provide a replacement if needed
   608  		load := *originalLoad
   609  		load.From = append([]*build.Ident{}, load.From...)
   610  		load.To = append([]*build.Ident{}, load.To...)
   611  
   612  		for i := 0; i < len(load.To); i++ {
   613  			from := load.From[i]
   614  			to := load.To[i]
   615  			// Check if the symbol was already loaded
   616  			origin, alreadyLoaded := loaded[to.Name]
   617  			start, _ := from.Span()
   618  			loaded[to.Name] = struct {
   619  				label, from string
   620  				line        int
   621  			}{load.Module.Token, from.Name, start.Line}
   622  
   623  			if alreadyLoaded {
   624  				// The same symbol has already been loaded earlier
   625  				if origin.label == load.Module.Token && origin.from == from.Name {
   626  					// Only fix if it's loaded from the same label and variable
   627  					load.To = append(load.To[:i], load.To[i+1:]...)
   628  					load.From = append(load.From[:i], load.From[i+1:]...)
   629  					i--
   630  					loadFindings = append(loadFindings, makeLinterFinding(to,
   631  						fmt.Sprintf("Symbol %q has already been loaded on line %d. Please remove it.", to.Name, origin.line)))
   632  					continue
   633  				}
   634  
   635  				loadFindings = append(loadFindings, makeLinterFinding(to,
   636  					fmt.Sprintf("A different symbol %q has already been loaded on line %d. Please use a different local name.", to.Name, origin.line)))
   637  				continue
   638  			}
   639  			_, ok := symbols[to.Name]
   640  			if !ok {
   641  				// Fallback to verify if the symbol is used as a type.
   642  				_, ok = types[to.Name]
   643  			}
   644  			if !ok && !edit.ContainsComments(originalLoad, "@unused") && !edit.ContainsComments(to, "@unused") && !edit.ContainsComments(from, "@unused") {
   645  				// The loaded symbol is not used and is not protected by a special "@unused" comment
   646  				load.To = append(load.To[:i], load.To[i+1:]...)
   647  				load.From = append(load.From[:i], load.From[i+1:]...)
   648  				i--
   649  
   650  				loadFindings = append(loadFindings, makeLinterFinding(to,
   651  					fmt.Sprintf("Loaded symbol %q is unused. Please remove it.\nTo disable the warning, add '@unused' in a comment.", to.Name)))
   652  				if f.Type == build.TypeDefault || f.Type == build.TypeBzl {
   653  					loadFindings[len(loadFindings)-1].Message += fmt.Sprintf(`
   654  If you want to re-export a symbol, use the following pattern:
   655  
   656      load(..., _%s = %q, ...)
   657      %s = _%s
   658  `, to.Name, from.Name, to.Name, to.Name)
   659  				}
   660  			}
   661  		}
   662  
   663  		if len(loadFindings) == 0 {
   664  			// No problems with the current load statement
   665  			continue
   666  		}
   667  
   668  		build.SortLoadArgs(&load)
   669  		var newStmt build.Expr = &load
   670  		if len(load.To) == 0 {
   671  			// If there are no loaded symbols left remove the entire load statement
   672  			newStmt = nil
   673  		}
   674  		replacement := LinterReplacement{&f.Stmt[stmtIndex], newStmt}
   675  
   676  		// Individual replacements can't be combined together: assume we need to remove both loaded
   677  		// symbols from
   678  		//
   679  		//     load(":foo.bzl", "a", "b")
   680  		//
   681  		// Individual replacements are just to remove each of the symbols, but if these replacements
   682  		// are applied together, the result will be incorrect and a syntax error in Bazel:
   683  		//
   684  		//     load(":foo.bzl")
   685  		//
   686  		// A workaround is to attach the full replacement to the first finding.
   687  		loadFindings[0].Replacement = []LinterReplacement{replacement}
   688  		findings = append(findings, loadFindings...)
   689  	}
   690  	return findings
   691  }
   692  
   693  // collectLocalVariables traverses statements (e.g. of a function definition) and returns a list
   694  // of idents for variables defined anywhere inside the function.
   695  func collectLocalVariables(stmts []build.Expr) []*build.Ident {
   696  	variables := []*build.Ident{}
   697  
   698  	for _, stmt := range stmts {
   699  		switch stmt := stmt.(type) {
   700  		case *build.DefStmt:
   701  			// Don't traverse nested functions
   702  		case *build.ForStmt:
   703  			variables = append(variables, bzlenv.CollectLValues(stmt.Vars)...)
   704  			variables = append(variables, collectLocalVariables(stmt.Body)...)
   705  		case *build.IfStmt:
   706  			variables = append(variables, collectLocalVariables(stmt.True)...)
   707  			variables = append(variables, collectLocalVariables(stmt.False)...)
   708  		case *build.AssignExpr:
   709  			variables = append(variables, bzlenv.CollectLValues(stmt.LHS)...)
   710  		}
   711  	}
   712  	return variables
   713  }
   714  
   715  // findUninitializedVariables takes a list of statements (e.g. body of a block statement)
   716  // and a map of previously initialized statements, and calls `callback` on all idents that are not
   717  // initialized. An ident is considered initialized if it's initialized by every possible execution
   718  // path (before or by `stmts`).
   719  // Returns a boolean indicating whether the current list of statements is guaranteed to be
   720  // terminated explicitly (by return or fail() statements) and a map of variables that are guaranteed
   721  // to be defined by `stmts`.
   722  func findUninitializedVariables(stmts []build.Expr, previouslyInitialized map[string]bool, callback func(*build.Ident)) (bool, map[string]bool) {
   723  	// Variables that are guaranteed to be initialized
   724  	locallyInitialized := make(map[string]bool) // in the local block of `stmts`
   725  	initialized := make(map[string]bool)        // anywhere before the current line
   726  	for key := range previouslyInitialized {
   727  		initialized[key] = true
   728  	}
   729  
   730  	// findUninitializedIdents traverses an expression (simple statement or a part of it), and calls
   731  	// `callback` on every *build.Ident that's not mentioned in the map of initialized variables
   732  	findUninitializedIdents := func(expr build.Expr, callback func(ident *build.Ident)) {
   733  		// Collect lValues, they shouldn't be taken into account
   734  		// For example, if the expression is `a = foo(b = c)`, only `c` can be an uninitialized variable here.
   735  		lValues := make(map[*build.Ident]bool)
   736  		build.WalkInterruptable(expr, func(expr build.Expr, stack []build.Expr) (err error) {
   737  			switch expr := expr.(type) {
   738  			case *build.DefStmt:
   739  				// Function arguments can't be uninitialized, even if they share the same
   740  				// name with a variable that's not initialized for some execution path
   741  				// in an outer scope.
   742  				for _, param := range expr.Params {
   743  					if ident, _ := build.GetParamIdent(param); ident != nil {
   744  						lValues[ident] = true
   745  					}
   746  				}
   747  				// Don't traverse into nested def statements
   748  				return &build.StopTraversalError{}
   749  			case *build.AssignExpr:
   750  				for _, ident := range bzlenv.CollectLValues(expr.LHS) {
   751  					lValues[ident] = true
   752  				}
   753  			}
   754  			return
   755  		})
   756  
   757  		// Check if the ident is really not initialized and call the callback on it.
   758  		callbackIfNeeded := func(ident *build.Ident) {
   759  			if !initialized[ident.Name] && !lValues[ident] {
   760  				callback(ident)
   761  			}
   762  		}
   763  
   764  		walkBlockList := map[build.Expr]bool{}
   765  		build.WalkInterruptable(expr, func(expr build.Expr, stack []build.Expr) (err error) {
   766  			if walkBlockList[expr] {
   767  				return &build.StopTraversalError{}
   768  			}
   769  			switch expr := expr.(type) {
   770  			case *build.DefStmt:
   771  				// The header of the DefStmt may contain uninitialized variables (e.g.
   772  				// default values of parameters) and should be traversed.
   773  				// Its body shouldn't be traversed because it has another scope and will
   774  				// be analyzed by another call of `findUninitializedVariables`.
   775  				for _, stmt := range expr.Body {
   776  					walkBlockList[stmt] = true
   777  				}
   778  			case *build.Comprehension, *build.LambdaExpr:
   779  				// Comprehension and Lambda nodes are special, they have their own scope
   780  				// with variables that are only defined inside.
   781  				// Instead of traversing inside stop the traversal and call a special
   782  				// function to retrieve idents from the outer scope that are used inside
   783  				// the comprehension.
   784  
   785  				_, used := extractIdentsFromStmt(expr)
   786  				for ident := range used {
   787  					callbackIfNeeded(ident)
   788  				}
   789  
   790  				return &build.StopTraversalError{}
   791  			case *build.Ident:
   792  				callbackIfNeeded(expr)
   793  			default:
   794  				// Just traverse further
   795  			}
   796  			return
   797  		})
   798  	}
   799  
   800  	for _, stmt := range stmts {
   801  		newlyDefinedVariables := make(map[string]bool)
   802  		switch stmt := stmt.(type) {
   803  		case *build.DefStmt:
   804  			// Don't traverse nested functions
   805  		case *build.CallExpr:
   806  			if _, ok := isFunctionCall(stmt, "fail"); ok {
   807  				return true, locallyInitialized
   808  			}
   809  		case *build.ReturnStmt:
   810  			findUninitializedIdents(stmt, callback)
   811  			return true, locallyInitialized
   812  		case *build.BranchStmt:
   813  			if stmt.Token == "break" || stmt.Token == "continue" {
   814  				return true, locallyInitialized
   815  			}
   816  		case *build.ForStmt:
   817  			// Although loop variables are defined as local variables, buildifier doesn't know whether
   818  			// the collection will be empty or not.
   819  
   820  			// Traverse but ignore the result. Even if something is defined inside a for-loop, the loop
   821  			// may be empty and the variable initialization may not happen.
   822  			findUninitializedIdents(stmt.X, callback)
   823  
   824  			// The loop can access the variables defined above, and also the for-loop variables.
   825  			initializedInLoop := make(map[string]bool)
   826  			for name := range initialized {
   827  				initializedInLoop[name] = true
   828  			}
   829  			for _, ident := range bzlenv.CollectLValues(stmt.Vars) {
   830  				initializedInLoop[ident.Name] = true
   831  			}
   832  			findUninitializedVariables(stmt.Body, initializedInLoop, callback)
   833  			continue
   834  		case *build.IfStmt:
   835  			findUninitializedIdents(stmt.Cond, callback)
   836  			// Check the variables defined in the if- and else-clauses.
   837  			terminatedTrue, definedInTrue := findUninitializedVariables(stmt.True, initialized, callback)
   838  			terminatedFalse, definedInFalse := findUninitializedVariables(stmt.False, initialized, callback)
   839  			if terminatedTrue && terminatedFalse {
   840  				return true, locallyInitialized
   841  			} else if terminatedTrue {
   842  				// Only take definedInFalse into account
   843  				for key := range definedInFalse {
   844  					locallyInitialized[key] = true
   845  					initialized[key] = true
   846  				}
   847  			} else if terminatedFalse {
   848  				// Only take definedInTrue into account
   849  				for key := range definedInTrue {
   850  					locallyInitialized[key] = true
   851  					initialized[key] = true
   852  				}
   853  			} else {
   854  				// If a variable is defined in both if- and else-clauses, it's considered as defined
   855  				for key := range definedInTrue {
   856  					if definedInFalse[key] {
   857  						locallyInitialized[key] = true
   858  						initialized[key] = true
   859  					}
   860  				}
   861  			}
   862  			continue
   863  		case *build.AssignExpr:
   864  			// Assignment expression. Collect all definitions from the lhs
   865  			for _, ident := range bzlenv.CollectLValues(stmt.LHS) {
   866  				newlyDefinedVariables[ident.Name] = true
   867  			}
   868  		}
   869  		findUninitializedIdents(stmt, callback)
   870  		for name := range newlyDefinedVariables {
   871  			locallyInitialized[name] = true
   872  			initialized[name] = true
   873  		}
   874  	}
   875  	return false, locallyInitialized
   876  }
   877  
   878  // uninitializedVariableWarning warns about usages of values that may not have been initialized.
   879  func uninitializedVariableWarning(f *build.File) []*LinterFinding {
   880  	findings := []*LinterFinding{}
   881  	build.WalkStatements(f, func(expr build.Expr, stack []build.Expr) (err error) {
   882  		def, ok := expr.(*build.DefStmt)
   883  		if !ok {
   884  			return
   885  		}
   886  
   887  		// Get all variables defined in the function body.
   888  		// If a variable is not defined there, it can be builtin, global, or loaded.
   889  		localVars := make(map[string]bool)
   890  		for _, ident := range collectLocalVariables(def.Body) {
   891  			localVars[ident.Name] = true
   892  		}
   893  
   894  		// Function parameters are guaranteed to be defined everywhere in the function, even if they
   895  		// are redefined inside the function body. They shouldn't be taken into consideration.
   896  		for _, param := range def.Params {
   897  			if name, _ := build.GetParamName(param); name != "" {
   898  				delete(localVars, name)
   899  			}
   900  		}
   901  
   902  		// Search for all potentially initialized variables in the function body
   903  		findUninitializedVariables(def.Body, make(map[string]bool), func(ident *build.Ident) {
   904  			// Check that the found ident represents a local variable
   905  			if localVars[ident.Name] {
   906  				findings = append(findings,
   907  					makeLinterFinding(ident, fmt.Sprintf(`Variable "%s" may not have been initialized.`, ident.Name)))
   908  			}
   909  		})
   910  		return
   911  	})
   912  	return findings
   913  }
   914  

View as plain text