From 186dedd4a94a8fd1e327ca5526e45045fd3afba0 Mon Sep 17 00:00:00 2001 From: Alex Torok Date: Fri, 11 Oct 2024 19:16:44 -0400 Subject: [PATCH] feat: warn when it looks like you're using ignore instead of exclude directive (#1955) **What type of PR is this?** Feature **What package or component does this PR mostly affect?** all **What does this PR do? Why is it needed?** `ignore` and `exclude` are somewhat confusing directive names, and are easily mixed up. Show a warning when using the `gazelle:ignore` directive with an argument. I've seen many places where people do something like `# gazelle:ignore testdata` and accidentally disable gazelle for the entire directory and subdirectories. This can be seen in [many public github repos](https://github.com/search?q=%2F%23+gazelle%3Aignore+.%2F+language%3AStarlark&type=code&l=Starlark). **Other notes for review** Add a new generated test directory to assert that the warning is shown correctly. --- tests/warns_on_invalid_directive_use/BUILD.in | 0 tests/warns_on_invalid_directive_use/BUILD.out | 0 tests/warns_on_invalid_directive_use/README.md | 1 + tests/warns_on_invalid_directive_use/WORKSPACE | 0 tests/warns_on_invalid_directive_use/arguments.txt | 2 ++ tests/warns_on_invalid_directive_use/expectedExitCode.txt | 1 + tests/warns_on_invalid_directive_use/expectedStderr.txt | 1 + .../warns_on_invalid_directive_use/ignore_directive/BUILD.in | 4 ++++ .../warns_on_invalid_directive_use/ignore_directive/BUILD.out | 4 ++++ walk/config.go | 3 +++ 10 files changed, 16 insertions(+) create mode 100644 tests/warns_on_invalid_directive_use/BUILD.in create mode 100644 tests/warns_on_invalid_directive_use/BUILD.out create mode 100644 tests/warns_on_invalid_directive_use/README.md create mode 100644 tests/warns_on_invalid_directive_use/WORKSPACE create mode 100644 tests/warns_on_invalid_directive_use/arguments.txt create mode 100644 tests/warns_on_invalid_directive_use/expectedExitCode.txt create mode 100644 tests/warns_on_invalid_directive_use/expectedStderr.txt create mode 100644 tests/warns_on_invalid_directive_use/ignore_directive/BUILD.in create mode 100644 tests/warns_on_invalid_directive_use/ignore_directive/BUILD.out diff --git a/tests/warns_on_invalid_directive_use/BUILD.in b/tests/warns_on_invalid_directive_use/BUILD.in new file mode 100644 index 000000000..e69de29bb diff --git a/tests/warns_on_invalid_directive_use/BUILD.out b/tests/warns_on_invalid_directive_use/BUILD.out new file mode 100644 index 000000000..e69de29bb diff --git a/tests/warns_on_invalid_directive_use/README.md b/tests/warns_on_invalid_directive_use/README.md new file mode 100644 index 000000000..eb7a1d587 --- /dev/null +++ b/tests/warns_on_invalid_directive_use/README.md @@ -0,0 +1 @@ +# Gazelle warns on invalid directive use diff --git a/tests/warns_on_invalid_directive_use/WORKSPACE b/tests/warns_on_invalid_directive_use/WORKSPACE new file mode 100644 index 000000000..e69de29bb diff --git a/tests/warns_on_invalid_directive_use/arguments.txt b/tests/warns_on_invalid_directive_use/arguments.txt new file mode 100644 index 000000000..06ec5178f --- /dev/null +++ b/tests/warns_on_invalid_directive_use/arguments.txt @@ -0,0 +1,2 @@ +-lang +nolang diff --git a/tests/warns_on_invalid_directive_use/expectedExitCode.txt b/tests/warns_on_invalid_directive_use/expectedExitCode.txt new file mode 100644 index 000000000..c22708346 --- /dev/null +++ b/tests/warns_on_invalid_directive_use/expectedExitCode.txt @@ -0,0 +1 @@ +0 \ No newline at end of file diff --git a/tests/warns_on_invalid_directive_use/expectedStderr.txt b/tests/warns_on_invalid_directive_use/expectedStderr.txt new file mode 100644 index 000000000..d05bbc226 --- /dev/null +++ b/tests/warns_on_invalid_directive_use/expectedStderr.txt @@ -0,0 +1 @@ +gazelle: the ignore directive does not take any arguments. Did you mean to use gazelle:exclude instead? in //ignore_directive '# gazelle:ignore *.go' diff --git a/tests/warns_on_invalid_directive_use/ignore_directive/BUILD.in b/tests/warns_on_invalid_directive_use/ignore_directive/BUILD.in new file mode 100644 index 000000000..dd13206fc --- /dev/null +++ b/tests/warns_on_invalid_directive_use/ignore_directive/BUILD.in @@ -0,0 +1,4 @@ +# gazelle:ignore *.go + +# valid use, but trailing whitespace should not warn +# gazelle:ignore diff --git a/tests/warns_on_invalid_directive_use/ignore_directive/BUILD.out b/tests/warns_on_invalid_directive_use/ignore_directive/BUILD.out new file mode 100644 index 000000000..dd13206fc --- /dev/null +++ b/tests/warns_on_invalid_directive_use/ignore_directive/BUILD.out @@ -0,0 +1,4 @@ +# gazelle:ignore *.go + +# valid use, but trailing whitespace should not warn +# gazelle:ignore diff --git a/walk/config.go b/walk/config.go index 4feceddea..a43912768 100644 --- a/walk/config.go +++ b/walk/config.go @@ -95,6 +95,9 @@ func (cr *Configurer) Configure(c *config.Config, rel string, f *rule.File) { } wcCopy.follow = append(wcCopy.follow, path.Join(rel, d.Value)) case "ignore": + if d.Value != "" { + log.Printf("the ignore directive does not take any arguments. Did you mean to use gazelle:exclude instead? in //%s '# gazelle:ignore %s'", f.Pkg, d.Value) + } wcCopy.ignore = true } }