Issues with classes starting with $

[snjeza]

Fixes #670

Signed-off-by: Snjezana Peco snjezana.peco@redhat.com

[snjeza]

@rgrunber I have rebased the PR.

[SentryMan]

Are the checks supposed to fail?

[iloveeclipse]

Are the checks supposed to fail?

No. There are 3 new compiler warnings https://ci.eclipse.org/jdt/job/eclipse.jdt.core-Github/job/PR-671/5/eclipse/

[robstryker]

@snjeza Maybe fix the NLS warning here?

"Virtual resource "+externalAnnotationPath+" cannot be used as annotationpath for project "+project.getName())); //$NON-NLS-1' //$NON-NLS-2$

Specifically, this line has $NON-NLS-1' when it shouldn't ?

[snjeza]

@robstryker @iloveeclipse I have updated the PR.

[SentryMan]

The tests pass, is there anything else that needs to be done before review?

[SentryMan]

Is there some other forum that should be used to request review?

[fbricon]

@iloveeclipse @stephan-herrmann @jukzi can a JDT committer give this PR some love pretty please?

[Copilot]

Pull request overview

This pull request fixes issues with Java classes whose names contain or start with the $ character, which were causing problems with code assistance, organize imports, search, and open type functionality.

Changes:

• Modified dollar sign replacement logic across multiple core classes to distinguish between $ as an inner class separator versus $ as part of a legitimate class name
• Added logic to preserve $ characters at the beginning/end of names and in consecutive sequences (e.g., $$)
• Fixed ClassFile.getTypeName() to handle classes starting with $ correctly by changing the condition from lastDollar > -1 to lastDollar > 0

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
ClassFileMatchLocator.java	Updated dollar-to-dot replacement logic for search functionality (has a critical bug)
Util.java	Added getLastDotDollar() helper and updated multiple signature processing methods to preserve $ in class names
NamedMember.java	Modified type name processing to preserve $ at boundaries and in consecutive sequences
NameLookup.java	Updated type lookup logic for both binary and source packages to handle $ correctly
LambdaExpression.java	Added isBinary() override to properly delegate to parent
ClassFile.java	Fixed getTypeName() to handle classes starting with $ by changing lastDollar comparison from > -1 to > 0
BinaryType.java	Changed declaring type check from lastDollar == -1 to lastDollar <= 0
Signature.java	Added getLastDotDollar() helper and updated multiple signature methods to preserve $ in class names
SignatureTests.java	Added comprehensive tests for classes with $ at start, end, and in consecutive sequences
ClassFileTests.java	Added integration tests for fully qualified names with various $ patterns

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The comment has a duplicate "see" - "see see https://...". Should be just "see https://...".

Suggested change

	// see see https://github.com/eclipse-jdt/eclipse.jdt.core/pull/671
	// see https://github.com/eclipse-jdt/eclipse.jdt.core/pull/671

[snjeza]

Fixed.

[Copilot]

The logic has a critical flaw. It only checks if the previous character is a dot to determine whether to replace '$' with '.'. This doesn't correctly handle consecutive dollar signs. For example, with input "a.b$$.C" (representing package a.b$$ with class C), the first '$' has previous='b' (not '.'), so it gets replaced with '.', making "a.b.$.C". But consecutive dollars should both be preserved as they're part of the package/class name. The logic needs to also check if the next character is a '$' before deciding to replace. Consider using a similar approach to the other files in this PR that check both isPreviousDollar and isNextDollar.

[snjeza]

b$$ doesn't start $

[Copilot]

There is significant code duplication for the logic that determines when to replace '$' with '.' (or vice versa). This same pattern appears in at least 8 different methods across Signature.java, Util.java, NameLookup.java, and NamedMember.java. Consider extracting this logic into a shared utility method to improve maintainability. A method like shouldReplaceDollarWithDot(char[] array, int index, int lastDot, int lastDollar) would reduce duplication and make the logic easier to maintain and test.

[Copilot]

There are multiple lines of commented-out code that should be removed before merging. Commented-out code makes the codebase harder to maintain and understand.

Suggested change

	// return this.typeName;
	// Internal class file name doesn't contain ".class" file extension

[snjeza]

Fixed

[Copilot]

Test is always true, because of this condition.
Test is always true, because of this condition.

[snjeza]

This is not correct.
See https://github.com/clembo590/issues/tree/vscode-java/issues/2219
An example

public class Main {
	public static void main(String[] args) {
		String name = "UndertowWebServerFactoryCustomizer$AbstractOptions";
		char[] chars = name.toCharArray();
		for (int i = 0; i < chars.length; i++) {
			boolean isPrevDollar = (i > 0) ? (chars[i - 1] == '$') : false;
			boolean isNextDollar = (i + 1 < chars.length) ? (chars[i + 1] == '$') : false;
			if (chars[i] == '$' && !isNextDollar && !isPrevDollar) {
				System.out.println("*** true" + " " + name + " chars[" + i + "]=" + chars[i] + " " + isPrevDollar + " "
						+ isNextDollar);
			} else {
				System.out.println("XXX false" + " " + name + " chars[" + i + "]=" + chars[i] + " " + isPrevDollar + " "
						+ isNextDollar);
			}
		}
	}
}

[iloveeclipse]

@fbricon : I've rebased on latest & asked Copilot for review. Looks like there is a room for improvement.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[iloveeclipse]

@snjeza : could you please squash two commits in one? Thanks.

[SentryMan]

perchance is there anything else needed?