//cmd/presubmit/presubmit.go: Limit tabs check to Python files only.
Change-Id: Ie11976f366be76a68577dbe94ec942699cf93c4a
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/577159
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Leandro Lovisolo <lovisolo@google.com>
diff --git a/cmd/presubmit/presubmit.go b/cmd/presubmit/presubmit.go
index d01f526..10d5183 100644
--- a/cmd/presubmit/presubmit.go
+++ b/cmd/presubmit/presubmit.go
@@ -88,7 +88,7 @@
ok := true
ok = ok && checkTODOHasOwner(ctx, changedFiles)
ok = ok && checkForStrayWhitespace(ctx, changedFiles)
- ok = ok && checkHasNoTabs(ctx, changedFiles)
+ ok = ok && checkPythonFilesHaveNoTabs(ctx, changedFiles)
ok = ok && checkBannedGoAPIs(ctx, changedFiles)
ok = ok && checkJSDebugging(ctx, changedFiles)
if !*commit {
@@ -450,18 +450,13 @@
return ok
}
-// checkHasNoTabs goes through all touched lines and returns false if any of them (barring
-// exceptions for Golang and Makefiles) have tabs anywhere.
+// checkPythonFilesHaveNoTabs goes through the touched lines of all Python files and returns false
+// if any of them have tabs anywhere.
// Based on https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/19b3eb5adbe00e9da40375cb5dc47380a46f3041/presubmit_canned_checks.py#441
-func checkHasNoTabs(ctx context.Context, files []fileWithChanges) bool {
- ignoreFileExts := []string{".go", ".mk"}
- ignoreFiles := []string{"Makefile"}
+func checkPythonFilesHaveNoTabs(ctx context.Context, files []fileWithChanges) bool {
ok := true
for _, f := range files {
- if contains(ignoreFiles, f.fileName) || contains(ignoreFiles, filepath.Base(f.fileName)) {
- continue
- }
- if contains(ignoreFileExts, filepath.Ext(f.fileName)) {
+ if filepath.Ext(f.fileName) != ".py" {
continue
}
for _, line := range f.touchedLines {
diff --git a/cmd/presubmit/presubmit_test.go b/cmd/presubmit/presubmit_test.go
index 4bb4d6b..4f1791d 100644
--- a/cmd/presubmit/presubmit_test.go
+++ b/cmd/presubmit/presubmit_test.go
@@ -450,11 +450,11 @@
`)
}
-func TestCheckHasNoTabs(t *testing.T) {
+func TestCheckPythonFilesHaveNoTabs(t *testing.T) {
test := func(name string, input []fileWithChanges, expectedReturn bool, expectedLogs string) {
t.Run(name, func(t *testing.T) {
ctx, logs := captureLogs()
- ok := checkHasNoTabs(ctx, input)
+ ok := checkPythonFilesHaveNoTabs(ctx, input)
assert.Equal(t, expectedReturn, ok)
assert.Equal(t, expectedLogs, logs.String())
})
@@ -480,7 +480,7 @@
},
}, true, "")
- test("Tabs ok in go or Makefiles", []fileWithChanges{
+ test("Tabs ok in non-Python files", []fileWithChanges{
{
fileName: "file.go",
touchedLines: []lineOfCode{{
@@ -495,9 +495,23 @@
num: 7,
}},
},
+ {
+ fileName: "file.ts",
+ touchedLines: []lineOfCode{{
+ contents: "\t// We're cool with tabs on TypeScript files",
+ num: 5,
+ }},
+ },
+ {
+ fileName: "foo/bar/README.md",
+ touchedLines: []lineOfCode{{
+ contents: "\tTabs are ok in markdown",
+ num: 6,
+ }},
+ },
}, true, "")
- test("Tabs not ok in 'normal' files", []fileWithChanges{
+ test("Tabs not ok in Python files", []fileWithChanges{
{
fileName: "file.py",
touchedLines: []lineOfCode{{
@@ -508,16 +522,8 @@
num: 5,
}},
},
- {
- fileName: "foo/bar/README.md",
- touchedLines: []lineOfCode{{
- contents: "\tNot ok in markdown",
- num: 7,
- }},
- },
}, false, `file.py:2 Tab character not allowed
file.py:5 Tab character not allowed
-foo/bar/README.md:7 Tab character not allowed
`)
}
