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 2.0: Wrong lookup for outer things #70216

Closed
vonagam opened this issue Dec 17, 2022 · 5 comments · Fixed by #70229 or #70246
Closed

GDScript 2.0: Wrong lookup for outer things #70216

vonagam opened this issue Dec 17, 2022 · 5 comments · Fixed by #70229 or #70246

Comments

@vonagam
Copy link
Contributor

vonagam commented Dec 17, 2022

Godot version

master

System information

MacOS 10.15.7

Issue description

#69587 introduced regression because of this line in reduce_identifier_from_base.

First argument here is parser->current_class, which ignores the fact that typing can happen in a different class, even in a different file. So lookup happens not form base perspective, but from perspective of a place of typing.

Example:

class A:
  class B:
    pass

class C:
  class D:
    func hello() -> void:
      print( A.B.D ) # no warning or error from analyzer here

Also get_class_node_current_scope_classes should not be used here as it collects both class, its base classes and outer classes together in one list, but reduce_identifier_from_base have different logic for base classes and outer classes. So outer logic is applied now to all base classes except first direct one.

Steps to reproduce

Copy paste code from the example and see that there is no warning in the editor.

Minimal reproduction project

70216.zip

@akien-mga
Copy link
Member

akien-mga commented Dec 17, 2022

CC @adamscott

@adamscott
Copy link
Member

Can you set this MRP in your description? I usually ask for users to join a MRP even if it's a simple copy and paste because it's way easier to know if an issue is fixed or not. Hence I asked you to open this issue, I created a MRP for you.

70216.zip

@vonagam
Copy link
Contributor Author

vonagam commented Dec 18, 2022

I mean, if it was that easy I would not have opened the issue.

Second part is important one.

Example:

class A:
  const Q := 'right one'

class X:
  const Q := 'wrong one'

class Y extends X:
  class B extends A:
    static func check() -> void:
      print(Q)

func _ready() -> void:
  Y.B.check()

Obviously, that Q from base class should have a priority over Q from base class of outer class, but it does not because of the wrong usage of get_class_node_current_scope_classes every class connected in any way to outer class gets checked even before first base class of the type in question is looked at. (So this is an easy example, you can come up with base class of outer class of base class of outer class and it will beat simple base class.)

@adamscott
Copy link
Member

@akien-mga Could you reopen the issue? The issue is not fixed, finally.

@akien-mga akien-mga reopened this Dec 18, 2022
@vonagam
Copy link
Contributor Author

vonagam commented Dec 18, 2022

Another issue because of this line:

class Hide: # Hide is needed just to separate from other issue
  class A: pass

class B extends Hide.A:
  static func check() -> void:
    print(B.A) # no warning. runtime error.
    print(B.B) # no warning. runtime error.

func _ready() -> void:
  B.check()

This logic should apply only if p_base is null (so on implicit instance).

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