These are chat archives for javaparser/javaparser/issue-101

18th
Feb 2015
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 21:46
Concerning the NameExpr/String mismatch:
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 21:58
e.g. I don't see why EnumConstantDeclaration uses a String for the name, whereas the MethodDeclaration does not.
Hm, when looking deeper into it, out MethodDeclaration in the .jj file seems to be incorrect: it allows names such as "foo.bar" - which is plain wrong
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:00
imho the only nice thing about NameExpr is that it has a position
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:01
Yeah, the method declaration of PMD's java file does not allow that, it just takes a simple identifier
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:01
which is lost if would use a simple String. I think for most usages that is useless, but for some (style refactoring?) it could be useful
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:01
that's true
whow, I don't get that gitter formatting. How does it order the chat messages?
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:02
I am seeing them in chronological orders (most recent at bottom)
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:02
bah, need to hit F5 in order to get a proper display :/
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:03
works for me (using Chrome)
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:03
using Chromium/Ubuntu
For me, it puts my messages on top of the last message, as if it would be one message
or somewhere else. whatever
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:04
I have version Version 40.0.2214.111 (64-bit)
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:04
So, about that positioning: the EnumConstantDeclaration starts with the name, or <IDENTIFIER>, and ends with whatever comes later
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:05
correct (which could be arguments and method declarations)
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:06
Ah, not true, it starts with the annotations that might be applied
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:06
good point
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:07
using version 39.0.2171.65 btw; so maybe that's it
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:07
I guess also comments could be applied to it (but it should not be so relevant)
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:07
yes... those are ignored by the ASTParser, right? Not sure how they fit in
so, for such elements, positioning is lost
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:08
the ASTParser treat them as whitespace, there is a second parser (CommentsParser) which parse them and use heuristics to associate them to the node being commented
but you can disable the whole thing if you are not interested in comments
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:09
that would mean that for those refactoring guys we should stick to the NameExpr and try to get rid of the String names
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:10
mmm
it could help maybe have a Node instead of a String (to know also the position), however I would not use NameExpr. To me NameExpr should be used for names inside expressions (like a variable name).
I mean, I am not sure exactly what NameExpr is supposed to be but that suffix definitely confuse me. I guess a node named Name would make more sense to me
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:14
Well, you just hit another inconsistency I feel
NameExpr is oftentimes used when not dealing with an expression
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:15
this sounds bad to me, it could be worthy to fix this
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:15
I mean, it can technically only be a java identifier
or, via its subclass QualifiedNameExpr, a concatenation of java identifiers
which is no "expression" after all
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:17
I guess so but, for example, looking for all nodes which implements Expression it would be found, and this is based
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:19
or, rather, it is no statement... wait
I'm mising things up
mxing
mixing, goddammit
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:19
no worries :)
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:19
should better go to bed ;)
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:20
we can also talk about that tomorrow, no hurry (I guess you are on the Berlin timezone, right?)
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:20
yes I am...
Dublin, is that correct? It's 22:15 there?
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:21
exactly :)
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:21
nice... more time for you still :)
OK, NameExpr makes sense for statements like int a = b;
so b is a NameExpr
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:22
that would make sense to me
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:22
it could also be 1 + 1
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:22
but a should not be a NameExpr
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:23
which would be a UnaryExpression or something, don't pin me down
that's where we need a NameExpr
yeah, a is a VariableDeclaratorId
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:24
guess so, and it makes sense to have a NameExpr because to understand what is really is (a reference to a local variable or a field) we should need to do names resolution, which we do not do
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:24
which has a private String name
currently, this makes the most sense to me
If we wanted to have the positioning information, we should not use NameExpr, but come up with another node
like Identifier or something
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:26
agreed
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:26
which it technically is... it may not sound so charming
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:27
in this way comments could be also associate directly to the name in some cases (corner cases, definitely a minor benefit)
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:27
that would mean we would need to adapt the .jj file and practically all the files that are NamedNodes
nice
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:28
that could take a while :) Luckily we have a good number of tests I think
however we can do that in steps maybe
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:32
yeah, this .jj stuff is a real fun
So as part of this issue?
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:33
it could make sense, maybe I could start publishing a first draft of the pull request after changing a few cases, assess it and proceed from there
Sebastian Kirsch (@skirsch79)
@sebastiankirsch
Feb 18 2015 22:34
sounds good
Federico Tomassetti
@ftomassetti
Feb 18 2015 22:35
cool