Bug 542657 - [navigation] Outline does not display correctly method parameter type containing $
Summary: [navigation] Outline does not display correctly method parameter type contain...
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 4.6   Edit
Hardware: All All
: P3 minor (vote)
Target Milestone: ---   Edit
Assignee: JDT-Text-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-11 06:02 EST by Marc Mazas CLA
Modified: 2023-01-07 17:21 EST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Mazas CLA 2018-12-11 06:02:02 EST
The outline view does not display the $ character in parameter types:
e.g.

class B {
  void m(C$a c) {
    return;
  }
  void n(C$ c) {
    return;
  }
  class C$a {
  }
  class C$ {
  }
}

Outline shows:
m(CA)
n(C)
C$A
C$
Comment 1 Stephan Herrmann CLA 2018-12-11 09:30:58 EST
Some confusion about '$' in binary names is unavoidable, because in binary form each name A$B could be a nested class with no easy means to distinguish from a toplevel class with '$' in its name.

Seeing the confusion in the Outline, however, looks unnecessary to me, since we obviously have the source type available.

OTOH, class names containing '$' should only be used in machine generated code, and then I wonder, how relevant it is to browse such code?
Comment 2 Marc Mazas CLA 2018-12-11 12:01:53 EST
I do not really understand your point. Say:

class B {
  void m(C$A c) {    return;  }
  void n(C$ c)  {    return;  }
  void o(c c)   {    return;  }
  void p(A c)   {    return;  }
  class A {  }
}
class B$A {} -> JDT shows an error: Type B$A is already defined
class C$A {}
class C$ {}
class C {}

If I comment line "class B$A {}", the java code is fine and LEGAL, the classes are B.class, B$A.class, C$.class, C$A.class
Outline shows
B
  m(CA) instead of m(C$A)
  n(C)  instead of expected n(C$): this C is not the same as the next in o(C)
  o(C)  which is expected
  p(A)  which is expected, not p(B$A)
C$A
C$

The example is perfectly legal and what is asked in the Outline is what will not lead to any confusion. No need to show p(B$A), P(A) as currently is fine.

Yes, the case happens with generated classes through the JTB tool, a JavaCC parser which produces visitors (interface and default implementation) whose methods have parameters of the generated classes types. Classes are generated from grammar productions, and for some productions "Prod" it generates 2 classes Prod and Prod$.
User subclasses the default visitors (which can have hundreds of methods) and therefore browse the code with these Prod and Prod$ types in the visit methods and so the Outline is very very useful.

Of course this is a minor problem.
Comment 3 Stephan Herrmann CLA 2018-12-11 14:44:29 EST
I was referring to the fact that a tool will inevitably be confused when it encounters references to a class of shape "A$B", when those references occur in a class file rather than source code. Just from the name it is unclear whether the UI should render this as "A$B" or "A.B", i.e., the token '$' is ambiguous, could be part of a name or the delimiter between outer and inner.

Showing CA instead of C$A is, however, wrong whichever way we look at it.


If we go by the letter, the JTB tool seems to encourage you to violate this from JLS ยง3.8: "The dollar sign should be used only in mechanically generated source code or, rarely, to access pre-existing names on legacy systems", by leaking such names into it's API. But this is no excuse for JDT simply swallowing '$' tokens.
Comment 4 Marc Mazas CLA 2018-12-12 04:47:28 EST
Ok, it is clear.
In fact, I was somewhat wrong with JTB: it does not add any trailing $, the user production name ended with a $.
So it is more to the JavaCC Eclipse plugin to discourage using $ in identifiers.
But also the JDT itself does not show any info/warning when an identifier (like for a member or a method) contains a $.
Comment 5 Eclipse Genie CLA 2020-12-02 00:14:18 EST
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 6 Eclipse Genie CLA 2023-01-07 17:21:13 EST
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.