[core] Add getXPathNodeName to the Node interface#879
Conversation
A default implementation is available in AbstractNode to preserve compatibility with the previous way, which used Object.toString. Fixes pmd#569
| @@ -132,9 +132,16 @@ protected boolean hasRealLoc() { | |||
|
|
|||
| @Override | |||
| public String toString() { | |||
There was a problem hiding this comment.
we should probably @Deprecated this implementation to be removed in PMD 7.0.0
| * <p>Please override it. It may be removed in a future major version. | ||
| */ | ||
| @Override | ||
| public String getXPathNodeName() { |
There was a problem hiding this comment.
I'd have this method at least log on WARNING to have people override this method. You can use the new PMDVersion.getNextMajor() to tell people to do so before 7.0.0 is out. We should also flag this method for removal on such release
|
|
||
| @Override | ||
| public String toString() { | ||
| return getXPathNodeName(); |
There was a problem hiding this comment.
We don't need this method. We would inherit it from AbstractNode and still want to kill it eventually
There was a problem hiding this comment.
Nice, it went right over my head lol
|
|
||
| @Override | ||
| public String toString() { | ||
| return getXPathNodeName(); |
| } | ||
|
|
||
| public String toString() { | ||
| return getXPathNodeName(); |
|
|
||
| public String toString() { | ||
| return PLSQLParserTreeConstants.jjtNodeName[id]; | ||
| return getXPathNodeName(); |
| } | ||
|
|
||
| public String toString() { | ||
| return getXPathNodeName(); |
|
|
||
| public String toString() { | ||
| return VmParserTreeConstants.jjtNodeName[id]; | ||
| return getXPathNodeName(); |
Log that the default implementation will be removed
jsotuyod
left a comment
There was a problem hiding this comment.
This is looking very neat. Kudos for taking the time to tackle it, and to @gibarsin and @MatiasComercio for coming up with the idea of decoupling these
| import net.sourceforge.pmd.lang.ast.AbstractNode; | ||
|
|
||
| public class AbstractJspNode extends AbstractNode implements JspNode { | ||
| public abstract class AbstractJspNode extends AbstractNode implements JspNode { |
There was a problem hiding this comment.
adding this abstract is a breaking API change. Even though I agree it makes perfect sense to have it, we can't break this API until PMD 7.0.0. We should revert this particular change. We could make a reminder ticket like we did with #463 for 6.0.0
| import net.sourceforge.pmd.lang.ast.AbstractNode; | ||
|
|
||
| public class AbstractVFNode extends AbstractNode implements VfNode { | ||
| public abstract class AbstractVFNode extends AbstractNode implements VfNode { |
A default implementation is available in AbstractNode to preserve compatibility with the previous way, which
used Object.toString.
Fixes #569