[core] Fix #3349 add source code in PMD output#3354
Conversation
- Configured CSV and XML renderers to include source code snippet
- Added XMLVerboseRenderer that can be called using "-f xml-verbose" which will include code snippet that was cause of the RuleViolation
oowekyala
left a comment
There was a problem hiding this comment.
Thanks for making changes, this looks better!
Generated by 🚫 Danger |
| public String getSourceCode(String encoding) { | ||
| if ("".equals(getFilename()) | ||
| || Files.notExists(Paths.get(getFilename()))) { | ||
| return null; |
There was a problem hiding this comment.
The fact that this method may return null should be documented
| protected String methodName = ""; | ||
| protected String variableName = ""; | ||
|
|
||
| public ParametricRuleViolation(Rule theRule, RuleContext ctx, T node, String message, String snippet) { |
There was a problem hiding this comment.
Nobody uses this constructor outside tests, so I don't think your feature works. Actually it looks like it's a real pain to support this as you'll have to change the RuleViolationFactory of every language. This is not a good idea since that has already been removed from pmd 7...
| @Override | ||
| public String getSourceCode(String encoding) { | ||
| try { | ||
| SourceCode sourceCode = new SourceCode(new SourceCode.StringCodeLoader(rawCodeSnippet)); |
There was a problem hiding this comment.
... The only reasonable way I see to implement this in pmd 6 is to use a FileCodeLoader here and load the filename, then slice the contents of the file. This should not be done eagerly as it sounds really bad for performance.
There was a problem hiding this comment.
I used StringCodeLoader here purely for testing purposes used by XMLVerboseRendererTest. Using FileCodeLoader caused an exception during testing.
There was a problem hiding this comment.
Yes but then your rawCodeSnippet is not the code snippet, it's the entire source file, since you slice it late. We can't keep all analysed files in memory just for this renderer to work...
There was a problem hiding this comment.
I intended my changes in ParametricRuleViolation class to be only used in unit testing. I can create a separate class for unit testing instead.
| return sourceCode.getSlice(getBeginLine(), getEndLine()); | ||
|
|
||
| } catch (RuntimeException e) { | ||
| return null; |
There was a problem hiding this comment.
I also wonder how much of a feature that is, if it's so fragile. Why would your code here fail?
There was a problem hiding this comment.
I had added this when I was using SourceCode.FileCodeLoader as it was causing an exception.
Describe the PR
code snippet when running PMD
Related issues
Ready?
./mvnw clean verifypasses (checked automatically by github actions)