Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: gazelle panics when "# gazelle:ignore" doesn't have a value #915

Merged
merged 5 commits into from
Dec 7, 2022

Conversation

stdll00
Copy link
Contributor

@stdll00 stdll00 commented Dec 6, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

If # gazelle:ignore line exists gazelle panics.

commit hash 6434e34 (added new test line)
//gazelle:gazelle_test failes

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //gazelle:gazelle_test
-----------------------------------------------------------------------------
--- FAIL: TestGazelleBinary (12.00s)
    --- FAIL: TestGazelleBinary/ignored_invalid_imported_module (0.38s)
        python_test.go:188: expected gazelle exit code: 0
            got: 2
        python_test.go:188: expected gazelle stderr: 
            got: panic: runtime error: index out of range [1] with length 1
            
            goroutine 1 [running]:
            github.com/bazelbuild/rules_python/gazelle.(*comment).asAnnotation(0x1004293c0?)
            	gazelle/parser.go:207 +0x13c
            github.com/bazelbuild/rules_python/gazelle.annotationsFromComments({0x14000031040, 0x4, 0x1f8?})
            	gazelle/parser.go:230 +0xac
            github.com/bazelbuild/rules_python/gazelle.(*python3Parser).parse(0x140001c6338, 0x14000010130)
            	gazelle/parser.go:131 +0x5a4
            github.com/bazelbuild/rules_python/gazelle.(*Python).GenerateRules(0x0?, {0x14000142370, {0x1400007a690, 0x6f}, {0x0, 0x0}, 0x140001a26e0, {0x0, 0x0, 0x0}, ...})
            	gazelle/generate.go:197 +0x68c
            main.runFixUpdate.func1({0x1400007a690, 0x6f}, {0x0, 0x0}, 0x14000142370, 0x0?, 0x140001a26e0, {0x0, 0x0, 0x0}, ...)
            	external/bazel_gazelle/cmd/gazelle/fix-update.go:298 +0xc88
            github.com/bazelbuild/bazel-gazelle/walk.Walk.func1(0x14000142370, {0x1400007a690, 0x6f}, {0x0, 0x0}, 0x0)
            	external/bazel_gazelle/walk/walk.go:179 +0x3ec
            github.com/bazelbuild/bazel-gazelle/walk.Walk(0x14000142370, {0x1400007e730?, 0x5, 0x5}, {0x140000694f0, 0x1, 0x1}, 0x0, 0x1400015f588)
            	external/bazel_gazelle/walk/walk.go:182 +0x190
            main.runFixUpdate({0x1400001e084, 0x6f}, 0x16fd42c0c?, {0x14000012050, 0x1, 0x1})
            	external/bazel_gazelle/cmd/gazelle/fix-update.go:275 +0x580
            main.run({0x1400001e084?, 0x6f?}, {0x14000012050?, 0x1?, 0x1?})
            	external/bazel_gazelle/cmd/gazelle/gazelle.go:95 +0x230
            main.main()
            	external/bazel_gazelle/cmd/gazelle/gazelle.go:72 +0xe4
            
        python_test.go:142: "" exists
        python_test.go:142: "/ignored_invalid_imported_module" exists
        python_test.go:142: "/ignored_invalid_imported_module/BUILD" exists
        python_test.go:142: "/ignored_invalid_imported_module/README.md" exists
        python_test.go:142: "/ignored_invalid_imported_module/WORKSPACE" exists
        python_test.go:142: "/ignored_invalid_imported_module/__init__.py" exists
        python_test.go:142: "/ignored_invalid_imported_module/gazelle_python.yaml" exists
        python_test.go:142: "/ignored_invalid_imported_module/test.yaml" exists
FAIL

What is the new behavior?

Ignores erorr. This is similar behavior if module name is invalid.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@stdll00 stdll00 requested a review from f0rmiga as a code owner December 6, 2022 09:05
@stdll00 stdll00 changed the title Small bugfix: Gazlle plugin panics when "# gazelle:ignore" line exists Small bugfix: Fix gazlle plugin panics when "# gazelle:ignore" line exists Dec 6, 2022
Copy link
Collaborator

@f0rmiga f0rmiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API is invalid. The # gazelle:ignore on a Python file requires a value. If you want to replace the panic with a more specific error, then you will have to change asAnnotation to also return an error.

@stdll00
Copy link
Contributor Author

stdll00 commented Dec 7, 2022

Thank you for review. Fixed to return the error.

Copy link
Collaborator

@f0rmiga f0rmiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for adding the test. Just check the suggestions before merging.

gazelle/parser.go Outdated Show resolved Hide resolved
gazelle/testdata/invalid_annotation/test.yaml Outdated Show resolved Hide resolved
stdll00 and others added 2 commits December 7, 2022 09:45
Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
@f0rmiga f0rmiga changed the title Small bugfix: Fix gazlle plugin panics when "# gazelle:ignore" line exists fix: gazelle panics when "# gazelle:ignore" doesn't have a value Dec 7, 2022
@f0rmiga f0rmiga merged commit 4528038 into bazelbuild:main Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants