Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions WARNINGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -554,15 +554,15 @@ Paths containing `/external/` (without a leading `//`) may indicate:
Examples that trigger this warning:

```python
srcs = ["/external/some_repo/file.h"]
data = ["path/external/repo/data.txt"]
copts = ["-iquote $(BINDIR)/external/some_repo/include"]
args = ["./external/repo/path/data.txt"]
```

Instead, use proper Bazel labels:

```python
srcs = ["@some_repo//file.h"]
data = ["@repo//path:data.txt"]
copts = ["-iquote $(execpath @some_repo//include:foo.h)/.."]
args = ["$(execpath @repo//path:data.txt)"]
```

Note: This warning does not apply to main repository paths like `//external/...`
Expand Down
28 changes: 28 additions & 0 deletions warn/warn_bazel.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,24 @@ func printWarning(f *build.File) []*LinterFinding {
return findings
}

func isDocStringContext(stringExpr *build.StringExpr, stack []build.Expr) bool {
if len(stack) == 0 {
return false
}

// Function docstring ("""...""")
if stringExpr.TripleQuote {
predecessor := stack[len(stack)-1]
if defStmt, ok := predecessor.(*build.DefStmt); ok {
if len(defStmt.Body) > 0 && defStmt.Body[0] == stringExpr {
return true
}
}
}

return false
}

func externalPathWarning(f *build.File) []*LinterFinding {
if f.Type == build.TypeDefault {
// Only applicable to Bazel files
Expand All @@ -269,6 +287,11 @@ func externalPathWarning(f *build.File) []*LinterFinding {
if !ok {
return
}

if isDocStringContext(stringExpr, stack) {
return
}

// Warn for "/external/" but not if "//" appears in the string (main repository paths)
if strings.Contains(stringExpr.Value, "/external/") && !strings.Contains(stringExpr.Value, "//") {
findings = append(findings,
Expand All @@ -291,6 +314,11 @@ func canonicalRepositoryWarning(f *build.File) []*LinterFinding {
if !ok {
return
}

if isDocStringContext(stringExpr, stack) {
return
}

if strings.Contains(stringExpr.Value, "@@") {
findings = append(findings,
makeLinterFinding(stringExpr, `String contains "@@" which indicates a canonical repository name reference that should be avoided.`))
Expand Down
36 changes: 33 additions & 3 deletions warn/warn_bazel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,20 @@ filegroup(
)

some_rule(
arg1 = "normal/path/file.txt",
arg1 = "normal/path/file.txt",
arg2 = "/external/repo/file.py",
arg3 = ["file1.txt", "/external/another/file.cc"],
)`,
)

def foo():
pass
"""unused triple-quote string with /external/"""
`,
[]string{
`:9: String contains "/external/" which may indicate a dependency on external repositories that could be fragile.`,
`:25: String contains "/external/" which may indicate a dependency on external repositories that could be fragile.`,
`:26: String contains "/external/" which may indicate a dependency on external repositories that could be fragile.`,
`31: String contains "/external/" which may indicate a dependency on external repositories that could be fragile.`,
},
scopeBazel)

Expand All @@ -260,6 +266,15 @@ py_binary(
)`,
[]string{},
scopeBazel)

// Test cases that should NOT warn (doc strings)
checkFindings(t, "external-path", `
def foo():
"""/external/"""
pass
`,
[]string{},
scopeBazel)
}

func TestCanonicalRepositoryWarning(t *testing.T) {
Expand All @@ -279,13 +294,28 @@ py_binary(
srcs = ["tool.py"],
data = ["@repo//file.txt"], # Should NOT warn (single @)
args = ["@@some_canonical_repo//path:target"],
)`,
)

def foo():
pass
"""unused triple-quote string with @@"""
`,
[]string{
`:1: String contains "@@" which indicates a canonical repository name reference that should be avoided.`,
`:3: String contains "@@" which indicates a canonical repository name reference that should be avoided.`,
`:7: String contains "@@" which indicates a canonical repository name reference that should be avoided.`,
`:8: String contains "@@" which indicates a canonical repository name reference that should be avoided.`,
`:15: String contains "@@" which indicates a canonical repository name reference that should be avoided.`,
`20: String contains "@@" which indicates a canonical repository name reference that should be avoided.`,
},
scopeBazel)

// Test cases that should NOT warn (doc strings)
checkFindings(t, "canonical-repository", `
def foo():
"""@@foo"""
pass
`,
[]string{},
scopeBazel)
}