Add a warning for invalid string sequences (bazelbuild#694)

WARNINGS.md

@@ -49,6 +49,7 @@ Warning categories supported by buildifier's linter:
   * [return-value](#return-value)
   * [rule-impl-return](#rule-impl-return)
   * [same-origin-load](#same-origin-load)
+  * [string-escape](#string-escape)
   * [string-iteration](#string-iteration)
   * [uninitialized](#uninitialized)
   * [unreachable](#unreachable)
@@ -792,6 +793,17 @@ load(":f.bzl", "s1", "s2")
 
 --------------------------------------------------------------------------------
 
+## <a name="string-escape"></a>Invalid escape sequence
+
+  * Category name: `string-escape`
+  * Flag in Bazel: [`--incompatible_restrict_string_escapes`](https://github.com/bazelbuild/bazel/issues/8380)
+  * Automatic fix: yes
+
+Unrecognized escape sequences in string literals (e.g. `"\a \b"` is error-prone and shouldn't
+be used. If you need the backslash symbol, escape it explicitly: `"\\a \\b"`.
+
+--------------------------------------------------------------------------------
+
 ## <a name="string-iteration"></a>String iteration is deprecated
 
   * Category name: `string-iteration`

warn/warn.go

@@ -146,6 +146,7 @@ var FileWarningMap = map[string]func(f *build.File) []*LinterFinding{
 	"rule-impl-return":          ruleImplReturnWarning,
 	"return-value":              missingReturnValueWarning,
 	"same-origin-load":          sameOriginLoadWarning,
+	"string-escape":             stringEscapeWarning,
 	"string-iteration":          stringIterationWarning,
 	"uninitialized":             uninitializedVariableWarning,
 	"unreachable":               unreachableStatementWarning,

warn/warn_operation.go

@@ -3,6 +3,9 @@
 package warn
 
 import (
+	"fmt"
+	"strings"
+
 	"github.com/bazelbuild/buildtools/build"
 )
 
@@ -108,3 +111,98 @@ func integerDivisionWarning(f *build.File) []*LinterFinding {
 	})
 	return findings
 }
+
+func stringEscapeWarning(f *build.File) []*LinterFinding {
+	var findings []*LinterFinding
+
+	build.WalkPointers(f, func(expr *build.Expr, stack []build.Expr) {
+		str, ok := (*expr).(*build.StringExpr)
+		if !ok {
+			return
+		}
+
+		var quotes, value string
+		if str.TripleQuote {
+			quotes = str.Token[:3]
+			value = str.Token[3 : len(str.Token)-3]
+		} else {
+			quotes = str.Token[:1]
+			value = str.Token[1 : len(str.Token)-1]
+		}
+
+		var problems []int  // positions of the problems (unidentified escape sequences)
+
+		escaped := false
+		// This for-loop doesn't correctly check for a backlash at the end of the string literal, but
+		// such string can't be parsed anyway, neither by Bazel nor by Buildifier.
+		for i, ch := range value {
+			if !escaped {
+				if ch == '\\' {
+					escaped = true
+				}
+				continue
+			}
+
+			switch ch {
+			case '\n', '\\', 'n', 'r', 't', 'x', '\'', '"', '0', '1', '2', '3', '4', '5', '6', '7':
+				// According to https://github.com/Quarz0/bazel/blob/207a6103393908aba64ddb96239fbdd56cdfec05/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
+				// \x is also included to the list, although it's not supported by Bazel, but it's supported
+				// by Buildifier. This is safe for the migration because it's never been supported in Bazel,
+				// even before --incompatible_restrict_string_escapes was flipped.
+			default:
+				problems = append(problems, i)
+			}
+			escaped = false
+		}
+
+		if len(problems) == 0 {
+			return
+		}
+
+		var msg string
+		if len(problems) == 1 {
+			msg = fmt.Sprintf(
+				"Invalid escape sequence \\%s at position %d.",
+				string(value[problems[0]]),
+				problems[0],
+				)
+		} else {
+			var builder strings.Builder
+			builder.WriteString("Invalid escape sequences:\n")
+			for _, pos := range problems {
+				builder.WriteString(fmt.Sprintf(
+					"    \\%s at position %d\n",
+					string(value[pos]),
+					pos,
+					))
+			}
+			msg = builder.String()
+		}
+		finding := makeLinterFinding(str, msg)
+
+		// Fix
+		var bytes []byte
+		index := 0
+		for _, backslashPos := range problems {
+			for ; index < backslashPos; index++ {
+				bytes = append(bytes, value[index])
+			}
+			bytes = append(bytes, '\\')
+		}
+		for ; index < len(value); index++ {
+			bytes = append(bytes, value[index])
+		}
+
+		token := quotes + string(bytes) + quotes
+		val, _, err := build.Unquote(token)
+		if err == nil {
+			newStr := *str
+			newStr.Token = token
+			newStr.Value = val
+			finding.Replacement = []LinterReplacement{{expr, &newStr}}
+		}
+
+		findings = append(findings, finding)
+	})
+	return findings
+}

warn/warn_operation_test.go

@@ -79,3 +79,61 @@ for x in l:
 		},
 		scopeEverywhere)
 }
+
+func TestStringEscape(t *testing.T) {
+	checkFindingsAndFix(t, "string-escape", `
+'foo'
+'\\foo\\"bar"\\'
+"\foo"
+'"\foo"\\\bar'
+
+'''
+"asdf"
+\a\b\c\d\e\f\g\h\i\j\k\l\m\n\o\p\q\r\s\t\u\v\w\x43\y\z\0\1\2\3\4\5\6\7\8\9
+'''
+`, `
+"foo"
+'\\foo\\"bar"\\'
+"\\foo"
+'"\\foo"\\\\bar'
+
+'''
+"asdf"
+\\a\\b\\c\\d\\e\\f\\g\\h\\i\\j\\k\\l\\m\n\\o\\p\\q\r\\s\t\\u\\v\\w\x43\\y\\z\0\1\2\3\4\5\6\7\\8\\9
+'''
+`,
+		[]string{
+			`:3: Invalid escape sequence \f at position 1.`,
+			`:4: Invalid escape sequences:
+    \f at position 2
+    \b at position 9
+`,
+			`:6: Invalid escape sequences:
+    \a at position 9
+    \b at position 11
+    \c at position 13
+    \d at position 15
+    \e at position 17
+    \f at position 19
+    \g at position 21
+    \h at position 23
+    \i at position 25
+    \j at position 27
+    \k at position 29
+    \l at position 31
+    \m at position 33
+    \o at position 37
+    \p at position 39
+    \q at position 41
+    \s at position 45
+    \u at position 49
+    \v at position 51
+    \w at position 53
+    \y at position 59
+    \z at position 61
+    \8 at position 79
+    \9 at position 81
+`,
+		},
+		scopeEverywhere)
+}