jamesladd / stc

stc
Other
3 stars 2 forks source link

Import/lookup uses incorrect class name and cannot resolve imports correctly #31

Closed mattys101 closed 6 years ago

mattys101 commented 6 years ago

The import code in RedlineSmalltalk appears to use the wrong classname. If I am reading it correctly the import structure is as follows:

package of importing class -> class doing the import -> short name of the imported class -> full name of imported class

To properly support this I think the addImport method needs to have a 4 arg signature like:

public void addImport(String packageName, String importingClassName, String className, String fullClassName)

rather than the current:

public void addImport(String packageName, String className, String fullClassName)

Which leads to

package of importing class -> imported class (short) name -> imported class (short) name -> full name of imported class

and hence the resolution of the import cannot find the imported class.

jamesladd commented 6 years ago

This may well be true.

You have the concept correctly, that there is a table if imports on a per class bases so MyClass can have imports and these can be very different to YourClass. Also, the idea is to allow you to import a class into your namespace and alias it.

Please can you tell me more about now the current structure is not finding classes?

mattys101 commented 6 years ago

If we have:

"some/package/MyClass.st"
Smalltalk import: 'some.other.package.SomeClass'.

SomeClass subclass: MyClass.

The import leads to:

some.package ->
    SomeClass ->
        SomeClass ->
            some.other.package.SomeClass

So when SomeClass is resolved in the context of MyClass the import cannot be found. Because it doesn't actually have any imports.

Does that help?

jamesladd commented 6 years ago

That does help. Thank you for such a detailed description of the problem and especially for the suggested fix. I have pushed a fix to master. Please let me know if this solves the issue. I did some testing but not as involved as you have.

mattys101 commented 6 years ago

Looks like that fixed it :-) Thanks.

jamesladd commented 6 years ago

Yay. Again thank you for such a detailed problem description and suggested fix.

Sent from my Commodore 64

On 24 Nov 2017, at 12:08 pm, Matt notifications@github.com wrote:

Looks like that fixed it :-) Thanks.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.