Importer: Missing information on class/enum field access

[Gabriel-Darbord]

Executing the following:

JavaSmaCCProgramNodeImporterVisitor new parseCodeString: 'class MyClass {
	static MyClass FOO = new MyClass();
	static void main() { return MyClass.FOO; }
}'

MyClass.FOO should yield a class field access, however we only obtain an identifier: FASTJavaIdentifier new name: 'FOO'.

[NicolasAnquetil]

Not sure: it is a class field access if you now that MyClass is a class, but the parser does not
abc.yyz could also be a fully qualified class name or an enum constant...

[Gabriel-Darbord]

Right, it could also refer to an enumeration. Is there any resolution done by the importer?
It can't be a qualified class name, because classes can't be used as variables by themselves. There must be a method call or attribute access to be a valid expression.

class MyClass {
  static MyClass getMyClass() { return MyClass; } // invalid, eclipse says "MyClass cannot be resolved to a variable"
}

[NicolasAnquetil]

nope, no resolution
parser is strictly syntax, replace all identifiers by abc, xyz, or _123 and you will get an idea what the parser sees.

And as for your example, yes, it could not be a fully qualified class, but it could be a variable "MyClass" with an attribute "FOO"

[Gabriel-Darbord]

Yes, it could be a variable access, which is what I initially wanted it to be when I opened the issue.
I understand that the importer doesn't do any resolution, and since it can be ambiguous, it just doesn't make a choice.
However, it's still missing something important. In the expression return MyClass.FOO, only FOO is recorded as an identifier, and the information about MyClass. is discarded, which is not OK.
The full information should be imported, and developers can do the resolution later with other tools if they wish.

[NicolasAnquetil]

hum right
so there are two strange things happening here:

• in JavaSmaCCProgramNodeImporterVisitor>>visitReturnStatement: there is a hack to change the visiting:

  (aReturnStatement expression isKindOf: JavaNameNode)
    ifTrue: [
      "If the element is named. It is more than propably an Identifier and not just an named element. So you pass over the normal rules of the importer"
     currentFASTEntity expression: (self addToModel: (self clone visitIdentifier: aReturnStatement expression))
    ]

and #visitIdentifier: does not consider qualified name

And if we want to remove this hack and just call self accept: aReturnStatement expression) (normal visit) then we get a second special case:

JavaSmaCCProgramNodeImporterVisitor >> visitQualifiedName: aQualifiedName
	self isOutsideTypeDeclaration
		ifTrue: [
			currentFASTEntity := self addToModel: FASTJavaQualifiedName new.
			self setStartEndPos: aQualifiedName.
			currentFASTEntity namespace: (self clone accept: aQualifiedName nspace).
			currentFASTEntity name: aQualifiedName name value ]
		ifFalse: [ 
			currentFASTEntity := self addToModel: FASTJavaClassProperty new.
			self setStartEndPos: aQualifiedName.
			currentFASTEntity type: (self clone accept: aQualifiedName nspace).
			currentFASTEntity fieldName: aQualifiedName name value ].
	^ currentFASTEntity

So it looks like somebody tried to make the visitor smarter than it should be and it ends up as more complicate than should.

I propose to remove all these special case and just parse everything as QualifiedName. But this will have impact and I don't want to deal with it just right now

[NicolasAnquetil]

hummm seems the meta-model is not right
last commit #e4e6724 removed first hack (in JavaSmaCCProgramNodeImporterVisitor>>visitReturnStatement:)

But the second hack (in JavaSmaCCProgramNodeImporterVisitor >> visitQualifiedName:) is based on some strange meta-model implementation of FASTJavaQualifiedName

This will need to be re-considered