diff --git a/build/BUILD.bazel b/build/BUILD.bazel index e1d3f6f12..aa1d1900c 100644 --- a/build/BUILD.bazel +++ b/build/BUILD.bazel @@ -33,6 +33,7 @@ go_test( size = "small", srcs = [ "checkfile_test.go", + "fstring_test.go", "lex_test.go", "parse_test.go", "print_test.go", diff --git a/build/fstring_test.go b/build/fstring_test.go new file mode 100644 index 000000000..275b52c4c --- /dev/null +++ b/build/fstring_test.go @@ -0,0 +1,144 @@ +/* +Copyright 2026 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package build + +import "testing" + +// F-strings are opaque text: {...} fields ride through in StringExpr.Value; +// the "f" prefix round-trips via StringExpr.Prefix. + +func TestFStringParsePrefixAndValue(t *testing.T) { + cases := []struct { + src string // single f-string expression + value string // expected StringExpr.Value + triple bool + prefix string + }{ + {`f"hello"`, "hello", false, "f"}, + {`f"{name}_lib"`, "{name}_lib", false, "f"}, + {`f"{prefix}_{suffix}_test"`, "{prefix}_{suffix}_test", false, "f"}, + {`f"//path/to:{target}"`, "//path/to:{target}", false, "f"}, + {`f"{ctx.label.name}.out"`, "{ctx.label.name}.out", false, "f"}, + {`f"got {value!r}, want {expected!r}"`, "got {value!r}, want {expected!r}", false, "f"}, + {`f"{count:>5d} items"`, "{count:>5d} items", false, "f"}, + {`f"{{not a field}}"`, "{{not a field}}", false, "f"}, + {`f'single {x}'`, "single {x}", false, "f"}, + {`f""" +error: {msg} + at {loc} +"""`, "\nerror: {msg}\n at {loc}\n", true, "f"}, + // Plain strings must not pick up a prefix. + {`"plain {x}"`, "plain {x}", false, ""}, + } + + for _, c := range cases { + f, err := ParseDefault("test.bzl", []byte("x = "+c.src+"\n")) + if err != nil { + t.Errorf("ParseDefault(%q) error: %v", c.src, err) + continue + } + assign, ok := f.Stmt[0].(*AssignExpr) + if !ok { + t.Errorf("%q: expected AssignExpr, got %T", c.src, f.Stmt[0]) + continue + } + s, ok := assign.RHS.(*StringExpr) + if !ok { + t.Errorf("%q: expected StringExpr RHS, got %T", c.src, assign.RHS) + continue + } + if s.Value != c.value { + t.Errorf("%q: Value = %q, want %q", c.src, s.Value, c.value) + } + if s.TripleQuote != c.triple { + t.Errorf("%q: TripleQuote = %v, want %v", c.src, s.TripleQuote, c.triple) + } + if s.Prefix != c.prefix { + t.Errorf("%q: Prefix = %q, want %q", c.src, s.Prefix, c.prefix) + } + } +} + +// "f" stays a valid identifier; only "f" immediately followed by a quote +// starts an f-string. +func TestFIdentifierNotPrefix(t *testing.T) { + src := `f = 1 + +def f(): + pass + +def g(f): + return f + 1 + +g(f = 42) +obj.f +` + if _, err := ParseDefault("test.bzl", []byte(src)); err != nil { + t.Errorf("expected 'f' to be a valid identifier in all positions, got: %v", err) + } +} + +func TestFStringPrintRoundTrip(t *testing.T) { + cases := []struct { + in string + want string + }{ + // Preserve as written. + {`f"{prefix}_{suffix}_test"`, `f"{prefix}_{suffix}_test"`}, + // No inner double quote: canonicalize to double quotes. + {`f'log({event})'`, `f"log({event})"`}, + // Inner double quote: keep single quotes. + {`f'{got!r} != "expected"'`, `f'{got!r} != "expected"'`}, + // Plain string with braces must not gain an f-prefix. + {`"%s does not interpolate {name}"`, `"%s does not interpolate {name}"`}, + } + for _, c := range cases { + f, err := ParseDefault("test.bzl", []byte("x = "+c.in+"\n")) + if err != nil { + t.Errorf("parse %q: %v", c.in, err) + continue + } + out := string(Format(f)) + want := "x = " + c.want + "\n" + if out != want { + t.Errorf("round-trip(%q):\n got: %q\n want: %q", c.in, out, want) + } + } +} + +// When Token can't be reused (e.g. programmatically constructed StringExpr), +// the printer must still emit the "f" prefix from Prefix and escape Value. +func TestFStringPrintFallback(t *testing.T) { + cases := []struct { + s *StringExpr + want string + }{ + {&StringExpr{Value: "{name}_test", Prefix: "f"}, `f"{name}_test"`}, + {&StringExpr{Value: `got "{x}"`, Prefix: "f"}, `f"got \"{x}\""`}, + {&StringExpr{Value: "err:\n {detail}", Prefix: "f", TripleQuote: true}, "f\"\"\"err:\n {detail}\"\"\""}, + {&StringExpr{Value: "{not_a_field}"}, `"{not_a_field}"`}, + {&StringExpr{Value: "{x}", Prefix: "f", Token: `"{x}"`}, `f"{x}"`}, + } + for _, c := range cases { + f := &File{Stmt: []Expr{c.s}} + out := string(Format(f)) + want := c.want + "\n" + if out != want { + t.Errorf("Format fallback:\n got: %q\n want: %q", out, want) + } + } +} diff --git a/build/lex.go b/build/lex.go index ace048add..ace39a0f0 100644 --- a/build/lex.go +++ b/build/lex.go @@ -549,7 +549,7 @@ func (in *input) Lex(val *yySymType) int { } return c - case 'r': // possible beginning of raw quoted string + case 'r', 'f': // possible raw- or f-string prefix if len(in.remaining) < 2 || in.remaining[1] != '"' && in.remaining[1] != '\'' { break } @@ -612,6 +612,10 @@ func (in *input) Lex(val *yySymType) int { } val.str = s val.triple = triple + val.prefix = "" + if len(val.tok) > 0 && (val.tok[0] == 'f' || val.tok[0] == 'r') { + val.prefix = string(val.tok[0]) + } return _STRING } diff --git a/build/parse.y b/build/parse.y index 588415c14..ddf0ca366 100644 --- a/build/parse.y +++ b/build/parse.y @@ -37,6 +37,7 @@ package build str string // decoding of quoted string pos Position // position of token triple bool // was string triple quoted? + prefix string // string-literal prefix (e.g. "f"), if any // partial syntax trees expr Expr @@ -994,6 +995,7 @@ string: Value: $1, TripleQuote: $1, End: $1.add($1), + Prefix: $1, Token: $1, } } diff --git a/build/parse.y.go b/build/parse.y.go index 9455c9ab5..6b9f938aa 100644 --- a/build/parse.y.go +++ b/build/parse.y.go @@ -13,6 +13,7 @@ type yySymType struct { str string // decoding of quoted string pos Position // position of token triple bool // was string triple quoted? + prefix string // string-literal prefix (e.g. "f"), if any // partial syntax trees expr Expr @@ -144,7 +145,7 @@ const yyEofCode = 1 const yyErrCode = 2 const yyInitialStackSize = 16 -//line build/parse.y:1059 +//line build/parse.y:1061 // Go helper code. @@ -1029,14 +1030,14 @@ yydefault: case 1: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:218 +//line build/parse.y:219 { yylex.(*input).file = &File{Stmt: yyDollar[1].exprs} return 0 } case 2: yyDollar = yyS[yypt-5 : yypt+1] -//line build/parse.y:225 +//line build/parse.y:226 { statements := yyDollar[4].exprs if yyDollar[2].exprs != nil { @@ -1058,20 +1059,20 @@ yydefault: } case 3: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:245 +//line build/parse.y:246 { yyVAL.exprs = yyDollar[1].exprs } case 6: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:253 +//line build/parse.y:254 { yyVAL.exprs = nil yyVAL.lastStmt = nil } case 7: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:258 +//line build/parse.y:259 { yyVAL.exprs = yyDollar[1].exprs yyVAL.lastStmt = yyDollar[1].lastStmt @@ -1085,21 +1086,21 @@ yydefault: } case 8: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:270 +//line build/parse.y:271 { yyVAL.exprs = yyDollar[1].exprs yyVAL.lastStmt = nil } case 9: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:276 +//line build/parse.y:277 { yyVAL.exprs = nil yyVAL.lastStmt = nil } case 10: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:281 +//line build/parse.y:282 { // If this statement follows a comment block, // attach the comments to the statement. @@ -1132,7 +1133,7 @@ yydefault: } case 11: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:312 +//line build/parse.y:313 { // Blank line; sever last rule from future comments. yyVAL.exprs = yyDollar[1].exprs @@ -1140,7 +1141,7 @@ yydefault: } case 12: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:318 +//line build/parse.y:319 { yyVAL.exprs = yyDollar[1].exprs yyVAL.lastStmt = yyDollar[1].lastStmt @@ -1154,14 +1155,14 @@ yydefault: } case 13: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:332 +//line build/parse.y:333 { yyVAL.exprs = yyDollar[1].exprs yyVAL.lastStmt = yyDollar[1].exprs[len(yyDollar[1].exprs)-1] } case 14: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:337 +//line build/parse.y:338 { yyVAL.exprs = []Expr{yyDollar[1].expr} yyVAL.lastStmt = yyDollar[1].expr @@ -1175,7 +1176,7 @@ yydefault: } case 15: yyDollar = yyS[yypt-5 : yypt+1] -//line build/parse.y:351 +//line build/parse.y:352 { yyVAL.def_header = &DefStmt{ Function: Function{ @@ -1189,14 +1190,14 @@ yydefault: } case 17: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:366 +//line build/parse.y:367 { yyDollar[1].def_header.Type = yyDollar[3].expr yyVAL.def_header = yyDollar[1].def_header } case 18: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:373 +//line build/parse.y:374 { yyDollar[1].def_header.Function.Body = yyDollar[3].exprs yyDollar[1].def_header.ColonPos = yyDollar[2].pos @@ -1205,7 +1206,7 @@ yydefault: } case 19: yyDollar = yyS[yypt-6 : yypt+1] -//line build/parse.y:380 +//line build/parse.y:381 { yyVAL.expr = &ForStmt{ For: yyDollar[1].pos, @@ -1217,14 +1218,14 @@ yydefault: } case 20: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:390 +//line build/parse.y:391 { yyVAL.expr = yyDollar[1].ifstmt yyVAL.lastStmt = yyDollar[1].lastStmt } case 21: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:398 +//line build/parse.y:399 { yyVAL.ifstmt = &IfStmt{ If: yyDollar[1].pos, @@ -1235,7 +1236,7 @@ yydefault: } case 22: yyDollar = yyS[yypt-5 : yypt+1] -//line build/parse.y:407 +//line build/parse.y:408 { yyVAL.ifstmt = yyDollar[1].ifstmt inner := yyDollar[1].ifstmt @@ -1254,7 +1255,7 @@ yydefault: } case 24: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:428 +//line build/parse.y:429 { yyVAL.ifstmt = yyDollar[1].ifstmt inner := yyDollar[1].ifstmt @@ -1267,26 +1268,26 @@ yydefault: } case 27: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:445 +//line build/parse.y:446 { yyVAL.exprs = append([]Expr{yyDollar[1].expr}, yyDollar[2].exprs...) yyVAL.lastStmt = yyVAL.exprs[len(yyVAL.exprs)-1] } case 28: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:451 +//line build/parse.y:452 { yyVAL.exprs = []Expr{} } case 29: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:455 +//line build/parse.y:456 { yyVAL.exprs = append(yyDollar[1].exprs, yyDollar[3].expr) } case 31: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:462 +//line build/parse.y:463 { yyVAL.expr = &ReturnStmt{ Return: yyDollar[1].pos, @@ -1295,7 +1296,7 @@ yydefault: } case 32: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:469 +//line build/parse.y:470 { yyVAL.expr = &ReturnStmt{ Return: yyDollar[1].pos, @@ -1303,25 +1304,25 @@ yydefault: } case 33: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:474 +//line build/parse.y:475 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 34: yyDollar = yyS[yypt-5 : yypt+1] -//line build/parse.y:475 +//line build/parse.y:476 { yyVAL.expr = binary(typed(yyDollar[1].expr, yyDollar[3].expr), yyDollar[4].pos, yyDollar[4].tok, yyDollar[5].expr) } case 35: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:476 +//line build/parse.y:477 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 36: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:478 +//line build/parse.y:479 { yyVAL.expr = &BranchStmt{ Token: yyDollar[1].tok, @@ -1330,7 +1331,7 @@ yydefault: } case 37: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:485 +//line build/parse.y:486 { yyVAL.expr = &BranchStmt{ Token: yyDollar[1].tok, @@ -1339,7 +1340,7 @@ yydefault: } case 38: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:492 +//line build/parse.y:493 { yyVAL.expr = &BranchStmt{ Token: yyDollar[1].tok, @@ -1348,13 +1349,13 @@ yydefault: } case 43: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:506 +//line build/parse.y:507 { yyVAL.expr = yyDollar[1].string } case 44: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:510 +//line build/parse.y:511 { yyVAL.expr = &DotExpr{ X: yyDollar[1].expr, @@ -1365,7 +1366,7 @@ yydefault: } case 45: yyDollar = yyS[yypt-8 : yypt+1] -//line build/parse.y:519 +//line build/parse.y:520 { load := &LoadStmt{ Load: yyDollar[1].pos, @@ -1381,7 +1382,7 @@ yydefault: } case 46: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:533 +//line build/parse.y:534 { yyVAL.expr = &CallExpr{ X: yyDollar[1].expr, @@ -1394,7 +1395,7 @@ yydefault: } case 47: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:544 +//line build/parse.y:545 { yyVAL.expr = &IndexExpr{ X: yyDollar[1].expr, @@ -1405,7 +1406,7 @@ yydefault: } case 48: yyDollar = yyS[yypt-6 : yypt+1] -//line build/parse.y:553 +//line build/parse.y:554 { yyVAL.expr = &SliceExpr{ X: yyDollar[1].expr, @@ -1418,7 +1419,7 @@ yydefault: } case 49: yyDollar = yyS[yypt-8 : yypt+1] -//line build/parse.y:564 +//line build/parse.y:565 { yyVAL.expr = &SliceExpr{ X: yyDollar[1].expr, @@ -1433,7 +1434,7 @@ yydefault: } case 50: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:577 +//line build/parse.y:578 { yyVAL.expr = &ListExpr{ Start: yyDollar[1].pos, @@ -1444,7 +1445,7 @@ yydefault: } case 51: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:586 +//line build/parse.y:587 { yyVAL.expr = &Comprehension{ Curly: false, @@ -1457,7 +1458,7 @@ yydefault: } case 52: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:597 +//line build/parse.y:598 { yyVAL.expr = &Comprehension{ Curly: true, @@ -1470,7 +1471,7 @@ yydefault: } case 53: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:608 +//line build/parse.y:609 { exprValues := make([]Expr, 0, len(yyDollar[2].kvs)) for _, kv := range yyDollar[2].kvs { @@ -1485,7 +1486,7 @@ yydefault: } case 54: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:621 +//line build/parse.y:622 { yyVAL.expr = &SetExpr{ Start: yyDollar[1].pos, @@ -1496,7 +1497,7 @@ yydefault: } case 55: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:630 +//line build/parse.y:631 { if len(yyDollar[2].exprs) == 1 && yyDollar[2].comma.Line == 0 { // Just a parenthesized expression, not a tuple. @@ -1518,49 +1519,49 @@ yydefault: } case 56: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:651 +//line build/parse.y:652 { yyVAL.exprs = nil } case 57: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:655 +//line build/parse.y:656 { yyVAL.exprs = yyDollar[1].exprs } case 58: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:661 +//line build/parse.y:662 { yyVAL.exprs = []Expr{yyDollar[2].expr} } case 59: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:665 +//line build/parse.y:666 { yyVAL.exprs = append(yyDollar[1].exprs, yyDollar[3].expr) } case 61: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:672 +//line build/parse.y:673 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 62: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:676 +//line build/parse.y:677 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, yyDollar[2].expr) } case 63: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:680 +//line build/parse.y:681 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, yyDollar[2].expr) } case 64: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:685 +//line build/parse.y:686 { yyVAL.loadargs = []*struct { from Ident @@ -1569,14 +1570,14 @@ yydefault: } case 65: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:689 +//line build/parse.y:690 { yyDollar[1].loadargs = append(yyDollar[1].loadargs, yyDollar[3].loadarg) yyVAL.loadargs = yyDollar[1].loadargs } case 66: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:695 +//line build/parse.y:696 { start := yyDollar[1].string.Start.add("'") if yyDollar[1].string.TripleQuote { @@ -1598,7 +1599,7 @@ yydefault: } case 67: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:712 +//line build/parse.y:713 { start := yyDollar[3].string.Start.add("'") if yyDollar[3].string.TripleQuote { @@ -1617,103 +1618,103 @@ yydefault: } case 68: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:727 +//line build/parse.y:728 { yyVAL.exprs = nil } case 69: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:731 +//line build/parse.y:732 { yyVAL.exprs = yyDollar[1].exprs } case 70: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:736 +//line build/parse.y:737 { yyVAL.exprs = nil } case 71: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:740 +//line build/parse.y:741 { yyVAL.exprs = yyDollar[1].exprs } case 72: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:746 +//line build/parse.y:747 { yyVAL.exprs = []Expr{yyDollar[1].expr} } case 73: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:750 +//line build/parse.y:751 { yyVAL.exprs = append(yyDollar[1].exprs, yyDollar[3].expr) } case 74: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:757 +//line build/parse.y:758 { yyVAL.exprs = []Expr{yyDollar[1].expr} } case 75: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:761 +//line build/parse.y:762 { yyVAL.exprs = append(yyDollar[1].exprs, yyDollar[3].expr) } case 77: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:768 +//line build/parse.y:769 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 78: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:772 +//line build/parse.y:773 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, yyDollar[2].expr) } case 79: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:776 +//line build/parse.y:777 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, nil) } case 80: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:780 +//line build/parse.y:781 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, yyDollar[2].expr) } case 82: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:789 +//line build/parse.y:790 { yyVAL.expr = typed(yyDollar[1].expr, yyDollar[3].expr) } case 83: yyDollar = yyS[yypt-5 : yypt+1] -//line build/parse.y:793 +//line build/parse.y:794 { yyVAL.expr = binary(typed(yyDollar[1].expr, yyDollar[3].expr), yyDollar[4].pos, yyDollar[4].tok, yyDollar[5].expr) } case 84: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:797 +//line build/parse.y:798 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, typed(yyDollar[2].expr, yyDollar[4].expr)) } case 85: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:801 +//line build/parse.y:802 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, typed(yyDollar[2].expr, yyDollar[4].expr)) } case 87: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:808 +//line build/parse.y:809 { tuple, ok := yyDollar[1].expr.(*TupleExpr) if !ok || !tuple.NoBrackets { @@ -1729,13 +1730,13 @@ yydefault: } case 88: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:823 +//line build/parse.y:824 { yyVAL.expr = nil } case 91: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:831 +//line build/parse.y:832 { yyVAL.expr = &LambdaExpr{ Function: Function{ @@ -1747,157 +1748,157 @@ yydefault: } case 92: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:840 +//line build/parse.y:841 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, yyDollar[2].expr) } case 93: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:841 +//line build/parse.y:842 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, yyDollar[2].expr) } case 94: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:842 +//line build/parse.y:843 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, yyDollar[2].expr) } case 95: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:843 +//line build/parse.y:844 { yyVAL.expr = unary(yyDollar[1].pos, yyDollar[1].tok, yyDollar[2].expr) } case 96: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:844 +//line build/parse.y:845 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 97: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:845 +//line build/parse.y:846 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 98: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:846 +//line build/parse.y:847 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 99: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:847 +//line build/parse.y:848 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 100: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:848 +//line build/parse.y:849 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 101: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:849 +//line build/parse.y:850 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 102: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:850 +//line build/parse.y:851 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 103: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:851 +//line build/parse.y:852 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 104: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:852 +//line build/parse.y:853 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 105: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:853 +//line build/parse.y:854 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 106: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:854 +//line build/parse.y:855 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 107: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:855 +//line build/parse.y:856 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 108: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:856 +//line build/parse.y:857 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 109: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:857 +//line build/parse.y:858 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, "not in", yyDollar[4].expr) } case 110: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:858 +//line build/parse.y:859 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 111: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:859 +//line build/parse.y:860 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 112: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:860 +//line build/parse.y:861 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 113: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:861 +//line build/parse.y:862 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 114: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:862 +//line build/parse.y:863 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 115: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:863 +//line build/parse.y:864 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 116: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:864 +//line build/parse.y:865 { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, yyDollar[2].tok, yyDollar[3].expr) } case 117: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:866 +//line build/parse.y:867 { if b, ok := yyDollar[3].expr.(*UnaryExpr); ok && b.Op == "not" { yyVAL.expr = binary(yyDollar[1].expr, yyDollar[2].pos, "is not", b.X) @@ -1907,7 +1908,7 @@ yydefault: } case 118: yyDollar = yyS[yypt-5 : yypt+1] -//line build/parse.y:874 +//line build/parse.y:875 { yyVAL.expr = &ConditionalExpr{ Then: yyDollar[1].expr, @@ -1919,55 +1920,55 @@ yydefault: } case 119: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:886 +//line build/parse.y:887 { yyVAL.exprs = []Expr{yyDollar[1].expr} } case 120: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:890 +//line build/parse.y:891 { yyVAL.exprs = append(yyDollar[1].exprs, yyDollar[3].expr) } case 121: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:895 +//line build/parse.y:896 { yyVAL.expr = nil } case 123: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:901 +//line build/parse.y:902 { yyVAL.exprs, yyVAL.comma = nil, Position{} } case 124: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:905 +//line build/parse.y:906 { yyVAL.exprs, yyVAL.comma = yyDollar[1].exprs, yyDollar[2].pos } case 125: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:915 +//line build/parse.y:916 { yyVAL.pos = Position{} } case 128: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:926 +//line build/parse.y:927 { yyVAL.pos = yyDollar[1].pos } case 129: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:934 +//line build/parse.y:935 { yyVAL.pos = Position{} } case 131: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:941 +//line build/parse.y:942 { yyVAL.kv = &KeyValueExpr{ Key: yyDollar[1].expr, @@ -1977,37 +1978,37 @@ yydefault: } case 132: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:951 +//line build/parse.y:952 { yyVAL.kvs = []*KeyValueExpr{yyDollar[1].kv} } case 133: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:955 +//line build/parse.y:956 { yyVAL.kvs = append(yyDollar[1].kvs, yyDollar[3].kv) } case 134: yyDollar = yyS[yypt-0 : yypt+1] -//line build/parse.y:960 +//line build/parse.y:961 { yyVAL.kvs = nil } case 135: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:964 +//line build/parse.y:965 { yyVAL.kvs = yyDollar[1].kvs } case 136: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:968 +//line build/parse.y:969 { yyVAL.kvs = yyDollar[1].kvs } case 138: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:975 +//line build/parse.y:976 { tuple, ok := yyDollar[1].expr.(*TupleExpr) if !ok || !tuple.NoBrackets { @@ -2023,49 +2024,50 @@ yydefault: } case 139: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:991 +//line build/parse.y:992 { yyVAL.string = &StringExpr{ Start: yyDollar[1].pos, Value: yyDollar[1].str, TripleQuote: yyDollar[1].triple, End: yyDollar[1].pos.add(yyDollar[1].tok), + Prefix: yyDollar[1].prefix, Token: yyDollar[1].tok, } } case 140: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:1003 +//line build/parse.y:1005 { yyVAL.expr = &Ident{NamePos: yyDollar[1].pos, Name: yyDollar[1].tok} } case 141: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:1009 +//line build/parse.y:1011 { yyVAL.expr = &LiteralExpr{Start: yyDollar[1].pos, Token: yyDollar[1].tok + "." + yyDollar[3].tok} } case 142: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:1013 +//line build/parse.y:1015 { yyVAL.expr = &LiteralExpr{Start: yyDollar[1].pos, Token: yyDollar[1].tok + "."} } case 143: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:1017 +//line build/parse.y:1019 { yyVAL.expr = &LiteralExpr{Start: yyDollar[1].pos, Token: "." + yyDollar[2].tok} } case 144: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:1021 +//line build/parse.y:1023 { yyVAL.expr = &LiteralExpr{Start: yyDollar[1].pos, Token: yyDollar[1].tok} } case 145: yyDollar = yyS[yypt-4 : yypt+1] -//line build/parse.y:1027 +//line build/parse.y:1029 { yyVAL.expr = &ForClause{ For: yyDollar[1].pos, @@ -2076,13 +2078,13 @@ yydefault: } case 146: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:1038 +//line build/parse.y:1040 { yyVAL.exprs = []Expr{yyDollar[1].expr} } case 147: yyDollar = yyS[yypt-3 : yypt+1] -//line build/parse.y:1042 +//line build/parse.y:1044 { yyVAL.exprs = append(yyDollar[1].exprs, &IfClause{ If: yyDollar[2].pos, @@ -2091,13 +2093,13 @@ yydefault: } case 148: yyDollar = yyS[yypt-1 : yypt+1] -//line build/parse.y:1051 +//line build/parse.y:1053 { yyVAL.exprs = yyDollar[1].exprs } case 149: yyDollar = yyS[yypt-2 : yypt+1] -//line build/parse.y:1055 +//line build/parse.y:1057 { yyVAL.exprs = append(yyDollar[1].exprs, yyDollar[2].exprs...) } diff --git a/build/print.go b/build/print.go index 0492dd434..a9ebf5760 100644 --- a/build/print.go +++ b/build/print.go @@ -626,11 +626,20 @@ func (p *printer) expr(v Expr, outerPrec int) { case *StringExpr: // If the Token is a correct quoting of Value and has double quotes, use it, // also use it if it has single quotes and the value itself contains a double quote symbol - // or if it's a raw string literal (starts with "r"). + // or if it's a raw string literal (starts with "r") or an f-string (starts with "f"). // This preserves the specific escaping choices that BUILD authors have made. + prefix := v.Prefix + if prefix == "" { + if strings.HasPrefix(v.Token, "r") { + prefix = "r" + } else if strings.HasPrefix(v.Token, "f") { + prefix = "f" + } + } + s, triple, err := Unquote(v.Token) if err == nil && s == v.Value && triple == v.TripleQuote { - if strings.HasPrefix(v.Token, `r`) { + if prefix == "r" && strings.HasPrefix(v.Token, `r`) { // Raw string literal token := v.Token if strings.HasSuffix(v.Token, `'`) && !strings.ContainsRune(v.Value, '"') { @@ -645,8 +654,23 @@ func (p *printer) expr(v Expr, outerPrec int) { break } + if prefix == "f" && strings.HasPrefix(v.Token, `f`) { + // f-string literal + token := v.Token + if strings.HasSuffix(v.Token, `'`) && !strings.ContainsRune(v.Value, '"') { + // Single quotes but no double quotes inside the string, replace with double quotes + if strings.HasSuffix(token, `'''`) { + token = `f"""` + token[4:len(token)-3] + `"""` + } else if strings.HasSuffix(token, `'`) { + token = `f"` + token[2:len(token)-1] + `"` + } + } + p.printf("%s", token) + break + } + // Non-raw string literal - if strings.HasPrefix(v.Token, `"`) || strings.ContainsRune(v.Value, '"') { + if prefix == "" && (strings.HasPrefix(v.Token, `"`) || strings.ContainsRune(v.Value, '"')) { // Either double quoted or there are double-quotes inside the string if IsCorrectEscaping(v.Token) { p.printf("%s", v.Token) @@ -655,7 +679,11 @@ func (p *printer) expr(v Expr, outerPrec int) { } } - p.printf("%s", quote(v.Value, v.TripleQuote)) + if prefix == "f" { + p.printf("f%s", quote(v.Value, v.TripleQuote)) + } else { + p.printf("%s", quote(v.Value, v.TripleQuote)) + } case *DotExpr: addParen(precSuffix) diff --git a/build/quote.go b/build/quote.go index 03bcdb6df..79312390f 100644 --- a/build/quote.go +++ b/build/quote.go @@ -86,11 +86,15 @@ var escapable = [256]bool{ // string value, whether the original was triple-quoted, and // an error describing invalid input. func Unquote(quoted string) (s string, triple bool, err error) { - // Check for raw prefix: means don't interpret the inner \. + // Check for a string prefix: "r" disables backslash interpretation; "f" + // marks an f-string whose {...} fields ride through as literal text. raw := false - if strings.HasPrefix(quoted, "r") { + switch { + case strings.HasPrefix(quoted, "r"): raw = true quoted = quoted[1:] + case strings.HasPrefix(quoted, "f"): + quoted = quoted[1:] } if len(quoted) < 2 { diff --git a/build/quote_test.go b/build/quote_test.go index 878d06d28..c477c1d23 100644 --- a/build/quote_test.go +++ b/build/quote_test.go @@ -130,6 +130,29 @@ func TestUnquote(t *testing.T) { } } +var fstringUnquoteTests = []struct { + q string // quoted source (with f prefix) + s string // expected unquoted value + triple bool +}{ + {`f""`, ``, false}, + {`f'single'`, `single`, false}, + {`f"\n{x}\t"`, "\n{x}\t", false}, + {`f"got \"{value}\""`, `got "{value}"`, false}, + {`f"""line1 +{x} +line3"""`, "line1\n{x}\nline3", true}, +} + +func TestUnquoteFString(t *testing.T) { + for _, tt := range fstringUnquoteTests { + s, triple, err := Unquote(tt.q) + if err != nil || s != tt.s || triple != tt.triple { + t.Errorf("Unquote(%s) = %#q, %v, %v; want %#q, %v, nil", tt.q, s, triple, err, tt.s, tt.triple) + } + } +} + func TestUnquoteErrors(t *testing.T) { for _, tt := range unquoteErrorTests { s, triple, err := Unquote(tt.q) diff --git a/build/rewrite.go b/build/rewrite.go index 75d498459..d16581e7c 100644 --- a/build/rewrite.go +++ b/build/rewrite.go @@ -560,9 +560,16 @@ func deduplicateStringList(x Expr) { // deduplicateStringExprs removes duplicate string expressions from a slice // without reordering its elements. // Any suffix-comments are lost, any before- and after-comments are preserved. +// F-strings are not duplicates of non-f-strings with the same value, since +// f"{x}" interpolates and "{x}" does not. Raw and non-raw strings with the +// same Value still dedupe (they represent the same runtime value). func deduplicateStringExprs(list []Expr) []Expr { + type key struct { + value string + isF bool + } var comments []Comment - alreadySeen := make(map[string]bool) + alreadySeen := make(map[key]bool) var deduplicated []Expr for _, value := range list { str, ok := value.(*StringExpr) @@ -570,15 +577,15 @@ func deduplicateStringExprs(list []Expr) []Expr { deduplicated = append(deduplicated, value) continue } - strVal := str.Value - if _, ok := alreadySeen[strVal]; ok { + k := key{str.Value, str.Prefix == "f"} + if _, ok := alreadySeen[k]; ok { // This is a duplicate of a string above. // Collect comments so that they're not lost. comments = append(comments, str.Comment().Before...) comments = append(comments, str.Comment().After...) continue } - alreadySeen[strVal] = true + alreadySeen[k] = true if len(comments) > 0 { comments = append(comments, value.Comment().Before...) value.Comment().Before = comments @@ -698,10 +705,12 @@ func sortStringExprs(list []Expr) []Expr { // uniq removes duplicates from a list, which must already be sorted. // It edits the list in place. +// f"x" and "x" are not duplicates (f-strings interpolate); raw and non-raw +// strings with the same value are duplicates (same runtime value). func uniq(sortedList []stringSortKey) []stringSortKey { out := sortedList[:0] for _, sk := range sortedList { - if len(out) == 0 || sk.value != out[len(out)-1].value { + if len(out) == 0 || sk.value != out[len(out)-1].value || sk.isF != out[len(out)-1].isF { out = append(out, sk) } } @@ -711,7 +720,7 @@ func uniq(sortedList []stringSortKey) []stringSortKey { // isUniq reports whether the sorted list only contains unique elements. func isUniq(list []stringSortKey) bool { for i := range list { - if i+1 < len(list) && list[i].value == list[i+1].value { + if i+1 < len(list) && list[i].value == list[i+1].value && list[i].isF == list[i+1].isF { return false } } @@ -728,6 +737,7 @@ type stringSortKey struct { phase int split []string value string + isF bool original int x Expr } @@ -735,6 +745,7 @@ type stringSortKey struct { func makeSortKey(index int, x *StringExpr) stringSortKey { key := stringSortKey{ value: x.Value, + isF: x.Prefix == "f", original: index, x: x, } @@ -776,6 +787,10 @@ func (x byStringExpr) Less(i, j int) bool { if xi.value != xj.value { return xi.value < xj.value } + if xi.isF != xj.isF { + // Non-f-strings sort before f-strings. + return !xi.isF + } return xi.original < xj.original } diff --git a/build/syntax.go b/build/syntax.go index e756ce7e2..d9e8bc48b 100644 --- a/build/syntax.go +++ b/build/syntax.go @@ -255,6 +255,11 @@ type StringExpr struct { TripleQuote bool // triple quote output End Position + // Prefix is the string-literal prefix ("r" or "f") populated by the lexer. + // Consumers can rely on this field uniformly; the printer additionally + // consults Token for round-trip preservation. + Prefix string + // To allow specific formatting of string literals, // at least within our requirements, record the // preferred form of Value. This field is a hint: diff --git a/warn/warn_control_flow.go b/warn/warn_control_flow.go index c1be14f7a..c815617a8 100644 --- a/warn/warn_control_flow.go +++ b/warn/warn_control_flow.go @@ -20,6 +20,7 @@ package warn import ( "fmt" + "regexp" "strings" "github.com/bazelbuild/buildtools/build" @@ -27,6 +28,98 @@ import ( "github.com/bazelbuild/buildtools/edit" ) +// fstringIdentRE matches identifier names inside f-string fields. +var fstringIdentRE = regexp.MustCompile(`[\p{L}_][\p{L}\p{N}_]*`) + +// fstringKeywords are reserved words to exclude when scanning f-string fields. +var fstringKeywords = map[string]bool{ + "and": true, + "as": true, + "break": true, + "continue": true, + "def": true, + "elif": true, + "else": true, + "False": true, + "for": true, + "if": true, + "in": true, + "is": true, + "lambda": true, + "None": true, + "not": true, + "or": true, + "pass": true, + "return": true, + "True": true, +} + +// extractIdentsFromFString returns identifier names referenced inside the +// {...} fields of an f-string value. +// +// Fields are not parsed; a regex pass extracts identifier-shaped tokens. +// Intentionally permissive: false positives only suppress unused-variable +// warnings (safe); false negatives would resurrect the regression where +// f-string references look unused. +func extractIdentsFromFString(value string) []string { + var idents []string + i := 0 + for i < len(value) { + off := strings.IndexByte(value[i:], '{') + if off < 0 { + return idents + } + j := i + off + // "{{" escapes a literal '{'. + if j+1 < len(value) && value[j+1] == '{' { + i = j + 2 + continue + } + // Find the matching '}', tracking nested {...} (allowed in format + // specs) and skipping braces inside string literals. + depth := 1 + k := j + 1 + var quote byte + for k < len(value) && depth > 0 { + c := value[k] + if quote != 0 { + if c == quote { + quote = 0 + } else if c == '\\' && k+1 < len(value) { + k++ // skip escaped char + } + } else { + switch c { + case '{': + depth++ + case '}': + depth-- + case '"', '\'': + quote = c + } + } + k++ + } + if depth != 0 { + // Unbalanced braces; stop extraction. + return idents + } + expr := value[j+1 : k-1] + for _, idx := range fstringIdentRE.FindAllStringIndex(expr, -1) { + // Skip attribute names: `bar` in `{foo.bar}` is not a variable. + if idx[0] > 0 && expr[idx[0]-1] == '.' { + continue + } + m := expr[idx[0]:idx[1]] + if !fstringKeywords[m] { + idents = append(idents, m) + } + } + i = k + } + return idents +} + // findReturnsWithoutValue searches for return statements without a value, calls `callback` on // them and returns whether the current list of statements terminates (either by a return or fail() // statements on the current level in all subbranches. @@ -380,6 +473,25 @@ func extractIdentsFromStmt(stmt build.Expr) (assigned, used map[*build.Ident]boo } used[expr] = true + case *build.StringExpr: + // Mark identifiers referenced inside f-string fields as used. + if expr.Prefix != "f" { + break + } + nextName: + for _, name := range extractIdentsFromFString(expr.Value) { + // Respect comprehension/lambda scopes, like the *Ident case. + for _, node := range stack { + if scope, ok := scopes[node]; ok { + if _, ok := scope[name]; ok { + continue nextName + } + } + } + // Consumers of `used` only read .Name; synthetic *Ident is fine. + used[&build.Ident{NamePos: expr.Start, Name: name}] = true + } + default: // Do nothing, just traverse further } diff --git a/warn/warn_control_flow_test.go b/warn/warn_control_flow_test.go index d6fcdff36..0a75eb054 100644 --- a/warn/warn_control_flow_test.go +++ b/warn/warn_control_flow_test.go @@ -701,6 +701,114 @@ def sample_macro_with_used_foo(name, foo = "foo"): inner_def() sample_macro_with_used_foo() +`, + []string{}, + scopeEverywhere) + + // `prefix` and `suffix` are used only inside an f-string. + checkFindings(t, "unused-variable", ` +def _make_lib(prefix, suffix): + full_name = f"{prefix}_{suffix}_lib" + native.cc_library(name = full_name) + +_make_lib("foo", "core") +`, + []string{}, + scopeEverywhere) + + // Comprehension iterator used only inside an f-string is still considered used. + checkFindings(t, "unused-variable", ` +def _deps_for(targets): + return [f"//external/{t}:lib" for t in targets] + +_deps_for(["a", "b"]) +`, + []string{}, + scopeEverywhere) + + // NEGATIVE: without the `f` prefix, {target} is literal text. + checkFindings(t, "unused-variable", ` +def _missing_prefix(): + target = "//foo:bar" + return "depends on {target}" + +_missing_prefix() +`, + []string{`:2: Variable "target" is unused.`}, + scopeEverywhere) + + // NEGATIVE: doubled braces {{...}} are literal text. + checkFindings(t, "unused-variable", ` +def _help_text(): + field = "..." + return f"see {{field}} in docs" + +_help_text() +`, + []string{`:2: Variable "field" is unused.`}, + scopeEverywhere) + + // NEGATIVE: comprehension iterator shadows outer `t`. + checkFindings(t, "unused-variable", ` +def _f(): + t = "outer" + return [f"//x/{t}:y" for t in items] + +_f() +`, + []string{`:2: Variable "t" is unused.`}, + scopeEverywhere) + + // NEGATIVE: lambda parameter shadows outer `x`. + checkFindings(t, "unused-variable", ` +def _g(): + x = "outer" + return lambda x: f"{x}" + +_g() +`, + []string{`:2: Variable "x" is unused.`}, + scopeEverywhere) + + // ACCEPTED FALSE-POSITIVE: the regex scanner can't tell a free variable + // from a single-letter format-spec character (`d` in `:>5d`), so `d` is + // conservatively treated as used. + checkFindings(t, "unused-variable", ` +def _format(count, d): + return f"{count:>5d}" + +_format(1, 2) +`, + []string{}, + scopeEverywhere) + + // Non-ASCII identifiers are recognized inside f-string fields. + checkFindings(t, "unused-variable", ` +def _greet(café): + return f"hello {café}" + +_greet("x") +`, + []string{}, + scopeEverywhere) + + // Attribute names after a dot are not variable references: `path` is the + // unused parameter, not the `ctx.label.path` attribute. + checkFindings(t, "unused-variable", ` +def _f(ctx, path): + return f"{ctx.label.path}" + +_f(None, "x") +`, + []string{`:1: Variable "path" is unused.`}, + scopeEverywhere) + + // Braces inside string literals don't terminate the field early. + checkFindings(t, "unused-variable", ` +def _f(prefix, suffix): + return f"{prefix + '}' + suffix}" + +_f("a", "b") `, []string{}, scopeEverywhere)