Skip to content

Fix GDScriptAnalyzer::is_shadowing() missing warnings for shadowed global constants#106984

Open
EdwardChanCH wants to merge 1 commit intogodotengine:masterfrom
EdwardChanCH:tooltip_local_var_hides_global
Open

Fix GDScriptAnalyzer::is_shadowing() missing warnings for shadowed global constants#106984
EdwardChanCH wants to merge 1 commit intogodotengine:masterfrom
EdwardChanCH:tooltip_local_var_hides_global

Conversation

@EdwardChanCH
Copy link
Copy Markdown
Contributor

@EdwardChanCH EdwardChanCH commented May 31, 2025

Closes: #117567

Related: #106987

Note: Some previous code has been split into #117545 to keep this PR manageable.

Any help is appreciated!

@EdwardChanCH EdwardChanCH requested a review from a team as a code owner May 31, 2025 05:31
@dalexeev dalexeev added this to the 4.5 milestone May 31, 2025
Comment thread modules/gdscript/gdscript_warning.h Outdated
Comment thread modules/gdscript/gdscript_analyzer.cpp Outdated
Comment thread modules/gdscript/gdscript_analyzer.cpp
@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch 2 times, most recently from a8e9f9a to 50855c4 Compare June 3, 2025 18:54
@EdwardChanCH EdwardChanCH requested a review from a team as a code owner June 3, 2025 18:54
@EdwardChanCH EdwardChanCH requested a review from dalexeev June 3, 2025 20:16
@EdwardChanCH EdwardChanCH marked this pull request as draft June 4, 2025 06:06
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

EdwardChanCH commented Jun 4, 2025

Converted to Draft PR, still discovering additional bugs to be fixed:

  • Local variables/ constants can shadow global enum classes or members
  • Local unnamed enum members can shadow global enum classes or members
  • Local named enum members have incorrect tooltips (and colours) if it is named PI, TAU, INF, or NAN
  • Local named enum classes can shadow global enum members when printed (e.g. enum OK { }, print(OK))
  • Local named enum classes can shadow global enum classes (e.g. Error.OK)
  • Local functions have incorrect tooltips (and colours) if it is named PI, TAU, INF, or NAN
  • Local functions can shadow global constants when printed (e.g. enum PI { }, print(PI))

Edit: Fixed.

@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch 7 times, most recently from 4736d02 to d007513 Compare June 5, 2025 06:10
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

EdwardChanCH commented Jun 5, 2025

Fixed all of the bugs listed above. Will add more test cases next.

Notes:

  • The [Ignore] warning button inserts @warning_ignore("shadowed_global_identifier") above/inside enum declarations, which produces parse compile errors.
    • A workaround is to surround the enum declaration with @warning_ignore_start("shadowed_global_identifier") + @warning_ignore_restore("shadowed_global_identifier").

The only notable side effect of the new enum warnings, unsure if it needs to be implemented in this PR.

Edit: This will be fixed in a future PR.

@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch 2 times, most recently from 61c3192 to 73cc4ff Compare June 6, 2025 00:34
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

Added more test cases and completed manual testing. Ready for review.

@EdwardChanCH EdwardChanCH marked this pull request as ready for review June 6, 2025 00:39
@EdwardChanCH EdwardChanCH requested a review from a team as a code owner June 6, 2025 00:39
@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch from 73cc4ff to a9f7543 Compare June 6, 2025 16:34
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

EdwardChanCH commented Jun 6, 2025

The unit tests all passed, but some leaked memory.

Edit: Fixed.

@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch 2 times, most recently from 1f2f570 to c330179 Compare June 9, 2025 00:26
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

EdwardChanCH commented Mar 12, 2026

I will refactor this PR soon, so this will be made a draft until then.

Edit: This PR now focuses on adding warnings for shadowed global constants.

Less relevant code has been moved to other PRs:
#117545 (fix tooltip for shadowed global constants)
#117308 (fix tooltip for shadowed global functions)
#106987 (add warnings for shadowed global functions)

@EdwardChanCH EdwardChanCH marked this pull request as draft March 12, 2026 00:42
@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch from 389a82b to d087b54 Compare March 17, 2026 16:26
@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch 2 times, most recently from d78cb1f to 8725c56 Compare March 18, 2026 00:16
@EdwardChanCH EdwardChanCH changed the title Fix GDScript tooltip showing shadowed global constants with warnings Fix GDScriptAnalyzer::is_shadowing() missing warning for shadowed global constants Mar 18, 2026
@EdwardChanCH EdwardChanCH changed the title Fix GDScriptAnalyzer::is_shadowing() missing warning for shadowed global constants Fix GDScriptAnalyzer::is_shadowing() missing warning for shadowed @GDScript constants Mar 18, 2026
@EdwardChanCH EdwardChanCH changed the title Fix GDScriptAnalyzer::is_shadowing() missing warning for shadowed @GDScript constants Fix GDScriptAnalyzer::is_shadowing() missing warnings for shadowed @GDScript constants Mar 18, 2026
@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch 2 times, most recently from 4386780 to bf336db Compare March 18, 2026 00:58
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

EdwardChanCH commented Mar 18, 2026

Below is a full summary of what this PR fixed, and what will be fixed in future issues/proposals:
godotengine/godot-proposals#14425

class_name _Main
extends Node

var PI = -10 # Missing warning. <-- Fixed! Now warns (SHADOWED_GLOBAL_IDENTIFIER)
var OK = -20 # Missing warning. <-- Fixed! Now warns (SHADOWED_GLOBAL_IDENTIFIER)
enum Error { # Missing warning. --> Proposal #14425
	OK = -30 # Missing warning. --> Proposal #14425
}
var Side = -40 # Missing warning. <-- Fixed! Now warns (SHADOWED_GLOBAL_IDENTIFIER)
var MouseButtonMask # Missing warning. <-- Fixed! Now warns (SHADOWED_GLOBAL_IDENTIFIER)
var number = -60 # No warning.

@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch 2 times, most recently from 50d9ab1 to 48559de Compare March 18, 2026 01:26
@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch from 48559de to 1eb0374 Compare March 18, 2026 01:29
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

Accidentally closed the PR...

@EdwardChanCH EdwardChanCH reopened this Mar 18, 2026
@EdwardChanCH EdwardChanCH marked this pull request as ready for review March 18, 2026 01:34
Comment thread modules/gdscript/gdscript_analyzer.cpp
Comment thread modules/gdscript/gdscript_analyzer.cpp Outdated
Comment thread modules/gdscript/gdscript_analyzer.cpp Outdated
@EdwardChanCH EdwardChanCH force-pushed the tooltip_local_var_hides_global branch from eef7141 to 06601b4 Compare March 20, 2026 16:13
@EdwardChanCH EdwardChanCH changed the title Fix GDScriptAnalyzer::is_shadowing() missing warnings for shadowed @GDScript constants Fix GDScriptAnalyzer::is_shadowing() missing warnings for shadowed global constants Mar 20, 2026
@EdwardChanCH EdwardChanCH requested a review from dalexeev March 20, 2026 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing warnings for shadowed @GDScript and @GlobalScope constants/enums

3 participants