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

Untyped Declaration warning for "for" iterator broken for dictionaries and can't be ignored #83660

Closed
Deozaan opened this issue Oct 20, 2023 · 3 comments

Comments

@Deozaan
Copy link

Deozaan commented Oct 20, 2023

Godot version

v4.2.beta2.official [f8818f8]

System information

Windows 10

Issue description

I like the new setting (from #81355) to warn for untyped declarations, but I am not liking it for for loops because it gets rid of the convenience of just having it inferred. Or at least it seems it can't be inferred for Dictionaries. Not only that, but adding @warning_ignore above the offending line of code does not make the warning go away.

So without a separate option to not warn for for loops, I'm tempted to just disable the whole thing.

Steps to reproduce

  1. Enable warnings for untyped declarations in the project settings.
  2. Create a script named mrp.gd with the following code:
extends Node

var alice = 5
var bob: Dictionary = {}

func _ready() -> void:
	for something in bob:
		pass
  1. Observe the following warnings:

[Ignore] Line 3 (UNTYPED_DECLARATION): Variable "alice" has no static type.
[Ignore] Line 7 (UNTYPED_DECLARATION): "for" iterator variable "something" has no static type.

  1. Click the [Ignore] link for both of the warnings to result in this code:
extends Node

@warning_ignore("untyped_declaration")
var alice = 5
var bob: Dictionary = {}

func _ready() -> void:
	@warning_ignore("untyped_declaration")
	for something in bob:
		pass
  1. Observe the following warning still persists:

[Ignore] Line 9 (UNTYPED_DECLARATION): "for" iterator variable "something" has no static type.

  1. Clicking [Ignore] repeatedly just adds another copy of "untyped_declaration" to the same line each time.
@warning_ignore("untyped_declaration", "untyped_declaration", "untyped_declaration")

While writing this up I discovered a similar issue for Arrays:

extends Node

@warning_ignore("untyped_declaration")
var alice = 5
var bob: Dictionary = {}
var carlos := []

func _ready() -> void:
	@warning_ignore("untyped_declaration")
	for something in bob:
		pass
	@warning_ignore("untyped_declaration")
	for other in carlos:
		pass

[Ignore]Line 10 (UNTYPED_DECLARATION):"for" iterator variable "something" has no static type.
[Ignore]Line 13 (UNTYPED_DECLARATION):"for" iterator variable "other" has no static type.

And just to be clear, the warning obviously goes away for the Array when I strongly type it and its elements, e.g., var carlos: Array[int] = [], but at that point the @warning_ignore is not needed to make it go away.

Minimal reproduction project

Shown above.

CC @ryanabx since I believe you implemented this feature, this issue may be of interest to you.

@dalexeev
Copy link
Member

Workaround:

for key: Variant in dictionary

Thanks for reporting the bug nonetheless!

@dalexeev dalexeev closed this as not planned Won't fix, can't repro, duplicate, stale Oct 20, 2023
@Deozaan
Copy link
Author

Deozaan commented Oct 21, 2023

I really wish there was a separate toggle for this feature in regard to for loops, as I can't picture myself ever wanting to explicitly declare the types for those variables rather than letting them just be inferred (or fallback to Variant for types that can't be strictly inferred, such as with Dictionaries). If the type can't be inferred from a Dictionary because a Dictionary can't be strongly typed then of course it's going to be Variant. So why not just automatically infer that and not nag about it?

I suppose the real solution to this problem is to add strongly typed Dictionaries so the type can be inferred in a for loop.

But if strongly typed Dictionaries or a separate option specifically for for loops are not going to happen (at least not anytime soon) then maybe I'll re-enable this feature once #83037 is merged so I can tell the compiler to ignore the warnings.

@dalexeev
Copy link
Member

We discussed this when implementing the warning. The problem is that for loop variable is different from other variables; you cannot tell it to infer its type or not, as you can for other variables (:= vs =). The loop variable always tries to infer the hard type if possible, and leaves the weak Varuant type if this is not possible. Accordingly, a warning for a loop variable is generated in the case of a weak type.

In the case of dictionaries, I think the bug is that keys and values are not inferred as hard Variants. If we change this, you can avoid specifying the type explicitly for dictionary keys without changing how the UNTYPED_DECLARATION warning works (but it still be generated if you're iterating over a value of an unknown type).

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

No branches or pull requests

2 participants