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

GDScript: Fix @warning_ignore annotation issues #83037

Merged

Conversation

dalexeev
Copy link
Member

@dalexeev dalexeev commented Oct 9, 2023


  • Fix warnings produced in the parser are not ignored since @warning_ignores are resolved and applied in the analyzer.
  • Fix annotations before blocks (if, for, while, etc.) applied to the first most nested statement due to a bug with recursion.
  • Fix parsing of standalone annotations in the script header.
  • Deprecate outdated and unused warnings PROPERTY_USED_AS_FUNCTION, CONSTANT_USED_AS_FUNCTION and FUNCTION_USED_AS_PROPERTY. In 4.x properties, methods and signals share the same name scope, accessing methods without parentheses returns a Callable.
    • These warnings have also been moved to the end of the list. This does not break compatibility since the enum is not exposed to the public API.
    • We could probably remove these warnings, since they are project settings (not real properties) and it is unlikely that anyone would use them.
  • Add missed STANDALONE_TERNARY (a special case that replaces STANDALONE_EXPRESSION, since the ternary operator can have side effects) and UNUSED_SIGNAL warnings.
  • Add support for @warning_ignore in match (to ignore the UNREACHABLE_PATTERN warning).
  • Add support for pass in match.

Production edit: closes godotengine/godot-roadmap#64

@dalexeev dalexeev added this to the 4.3 milestone Oct 9, 2023
@dalexeev dalexeev force-pushed the gds-fix-warning-ignore-issues branch 3 times, most recently from 40936bf to b2b759c Compare October 9, 2023 10:24
@dalexeev dalexeev force-pushed the gds-fix-warning-ignore-issues branch from b2b759c to 2cb231a Compare December 25, 2023 06:20
@dalexeev dalexeev force-pushed the gds-fix-warning-ignore-issues branch 2 times, most recently from db9580a to 705fb79 Compare February 4, 2024 10:31
@dalexeev dalexeev marked this pull request as ready for review February 4, 2024 10:34
@dalexeev dalexeev requested review from a team as code owners February 4, 2024 10:34
@dalexeev dalexeev force-pushed the gds-fix-warning-ignore-issues branch 2 times, most recently from 5baf480 to c0b4016 Compare February 4, 2024 19:58
@adamscott adamscott requested a review from vnen February 4, 2024 21:03
Copy link
Member

@adamscott adamscott left a comment

Choose a reason for hiding this comment

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

Nice PR. Seems to fix a lot of issues. Maybe we could merge this PR before dev4, as it changes a lot of lines? cc. @akien-mga

modules/gdscript/gdscript_parser.cpp Outdated Show resolved Hide resolved
modules/gdscript/gdscript_parser.cpp Outdated Show resolved Hide resolved
modules/gdscript/gdscript_parser.cpp Outdated Show resolved Hide resolved
modules/gdscript/gdscript_warning.cpp Outdated Show resolved Hide resolved
@dalexeev dalexeev force-pushed the gds-fix-warning-ignore-issues branch from c0b4016 to 11ee750 Compare February 5, 2024 05:39
@dalexeev dalexeev force-pushed the gds-fix-warning-ignore-issues branch from 11ee750 to b5e409b Compare February 6, 2024 07:25
@akien-mga akien-mga requested a review from adamscott February 14, 2024 08:41
@dalexeev dalexeev force-pushed the gds-fix-warning-ignore-issues branch from b5e409b to e70cdab Compare February 28, 2024 14:24
@akien-mga
Copy link
Member

Needs resyncing the docs for the is_deprecated -> deprecated change.

doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved
doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved
doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved
@dalexeev dalexeev force-pushed the gds-fix-warning-ignore-issues branch from e70cdab to c0ef2b0 Compare February 28, 2024 14:46
Copy link
Member

@adamscott adamscott left a comment

Choose a reason for hiding this comment

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

Reviewed during the GDScript meeting. Other than small nitpicks, the PR seems absolutely fine! Thanks for your hard work, @dalexeev.

@dalexeev dalexeev force-pushed the gds-fix-warning-ignore-issues branch from c0ef2b0 to ef1909f Compare March 12, 2024 16:00
@akien-mga akien-mga merged commit 8620f73 into godotengine:master Mar 13, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@dalexeev dalexeev deleted the gds-fix-warning-ignore-issues branch March 13, 2024 17:47
@MikeSchulze
Copy link

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants