search

eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
165 stars 130 forks source link

ClassFileWorkingCopy.getFileName() does not follow spec of IDependent #3215

Open robstryker opened 1 week ago

robstryker commented 1 week ago

ClassFileWorkingCopy extends CompilationUnit, which implements org.eclipse.jdt.internal.compiler.env.ICompilationUnit which implements IDependent.

IDependent has following method: char[] getFileName();

The javadoc on this method indicates getFileName() should return a string like /lib/some.zip|/com/q/Y.class

CompilationUnit (and it's subclass, ClassFileWorkingCopy) returns the following for this method call: getPath().toString().toCharArray();

getPath() is from IJavaElement, and says it must return basically the path to the archive itself, without the pipe character and package and class details within the archive.

With this in mind, if a ClassFileWorkingCopy is from inside an archive, it should not be using getPath().toString().toCharArray() as its getFileName() implementation, because this will only return the path to the archive, and leave out the remaining requirements from IDependent.getFileName(). 

// BIG PROBLEM HERE!!!
// IJavaElement#getPath() has different specs than
// org.eclipse.jdt.internal.compiler.env.IDependent#getFileName()
stephan-herrmann commented 1 week ago

Interesting find. Did you observe and bad effects of this difference?

And while I'm at it: a unit test for your PR would be awesome :)

robstryker commented 1 week ago

I've added a unit test. Unfortunately I added it to the JavaSearch suite, because that's where I was experiencing the error myself.

Basically, if these working copies return the jar path rather than the full path/to/jar|pack/class.class path, the ast parser will get confused which source leads to which output when using three classes within the same jar.

The test provided will try to create ClassFileWorkingCopy objects out of 3 classes in java.lang.etc, and then parse them via ASTParser.

There were also other symptoms that appeared later, but I don't remember them very much.