Skip to content

Tweak new signature logic that checks context of dollar#5073

Merged
iloveeclipse merged 2 commits into
eclipse-jdt:masterfrom
jjohnstn:dollarfix2
May 20, 2026
Merged

Tweak new signature logic that checks context of dollar#5073
iloveeclipse merged 2 commits into
eclipse-jdt:masterfrom
jjohnstn:dollarfix2

Conversation

@jjohnstn
Copy link
Copy Markdown
Contributor

@jjohnstn jjohnstn commented May 19, 2026

  • modify Signature.appendClassTypeSignature() logic for the
    last segment to use the old logic as it appears there is other
    code that is not prepared for the dollar being part of the class
    name and expects the dollar being converted to dot

What it does

See commit message.

How to test

See #5064 comments.

Author checklist

- modify Signature.appendClassTypeSignature() logic for the
  last segment to use the old logic as it appears there is other
  code that is not prepared for the dollar being part of the class
  name and expects the dollar being converted to dot
@iloveeclipse
Copy link
Copy Markdown
Member

  • it appears there is other
    code that is not prepared for the dollar being part of the class
    name and expects the dollar being converted to dot

Could you please give more insights? Which code exactly, in JDT or Xtext?

@iloveeclipse
Copy link
Copy Markdown
Member

Is it possible to add test for this?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts Signature.appendClassTypeSignature()’s handling of '$' in resolved class type signatures, aiming to restore the pre-#5064 behavior for the last segment (converting '$' to '.') to maintain compatibility with existing downstream code that expects that conversion.

Changes:

  • Updated the C_DOLLAR handling in Signature.appendClassTypeSignature() to keep '$' when it appears to be part of a non-terminal segment, but to apply the old '$''.' conversion when it appears to be in the last segment.
  • Introduced a lookahead scan to detect whether a '.' appears after '$' in certain contexts.

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

Comment thread org.eclipse.jdt.core/model/org/eclipse/jdt/core/Signature.java
Comment thread org.eclipse.jdt.core/model/org/eclipse/jdt/core/Signature.java
@jjohnstn
Copy link
Copy Markdown
Contributor Author

  • it appears there is other
    code that is not prepared for the dollar being part of the class
    name and expects the dollar being converted to dot

Could you please give more insights? Which code exactly, in JDT or Xtext?

In Javadoc of Signature.toCharArray():

Note: This method assumes that a type signature containing a '$'

  • is an inner type signature. While this is correct in most cases, someone could
  • define a non-inner type name containing a '$'. Handling this
  • correctly in all cases would have required resolving the signature, which
  • generally not feasible.

I will add a test to SignatureTests.

@jjohnstn
Copy link
Copy Markdown
Contributor Author

@iloveeclipse Just to confirm that the two XTest startswithdollar tests run succesfully with this change.

@cdietrich
Copy link
Copy Markdown

can confirm this would make our test green again / restore the old behaviour

@iloveeclipse
Copy link
Copy Markdown
Member

Thanks Jeff for the fast fix & Christian for verification. Looks good, let's merge.

@iloveeclipse iloveeclipse merged commit 247b3db into eclipse-jdt:master May 20, 2026
12 of 13 checks passed
@jjohnstn jjohnstn added this to the 4.40 RC1 milestone May 21, 2026
@jjohnstn jjohnstn self-assigned this May 22, 2026
@jjohnstn jjohnstn added the regression Something was broken by a previous change label May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

regression Something was broken by a previous change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants