phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve jjt error messages

Browse Source
This commit is contained in:
Clément Fournier
2023-03-19 22:49:45 +01:00
parent a4024c0021
commit 37e2f8f208
16 changed files with 105 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
javacc-wrapper.xml
View File

@ -246,6 +246,11 @@
<fileset file="${parser-file}" />
</replaceregexp>
<replaceregexp flags="g">
<regexp pattern="throw new ParseException\(\);" />
<substitution expression='throw net.sourceforge.pmd.util.AssertionUtil.shouldNotReachHere("consumetoken(-1) should have thrown");' />
<fileset file="${parser-file}" />
</replaceregexp>
<replaceregexp>
<regexp pattern="public interface"/>

4
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexParser.java
View File

@ -9,6 +9,7 @@ import net.sourceforge.pmd.lang.apex.ApexJorjeLogging;
import net.sourceforge.pmd.lang.apex.ApexLanguageProcessor;
import net.sourceforge.pmd.lang.ast.ParseException;
import net.sourceforge.pmd.lang.ast.Parser;
import net.sourceforge.pmd.lang.document.FileLocation;
import apex.jorje.data.Locations;
import apex.jorje.semantic.ast.compilation.Compilation;
@ -32,7 +33,8 @@ public final class ApexParser implements Parser {
final ApexTreeBuilder treeBuilder = new ApexTreeBuilder(task, (ApexLanguageProcessor) task.getLanguageProcessor());
return treeBuilder.buildTree(astRoot);
} catch (apex.jorje.services.exception.ParseException e) {
throw new ParseException(e).setFileName(task.getTextDocument().getPathId());
FileLocation loc = FileLocation.caret(task.getTextDocument().getPathId(), e.getLoc().getLine(), e.getLoc().getColumn());
throw new ParseException(e).withLocation(loc);
}
}
}

2
pmd-core/src/main/java/net/sourceforge/pmd/cpd/internal/JavaCCTokenizer.java
View File

@ -58,7 +58,7 @@ public abstract class JavaCCTokenizer implements Tokenizer {
currentToken = tokenFilter.getNextToken();
}
} catch (FileAnalysisException e) {
throw e.setFileName(textFile.getPathId());
throw e.setFileId(textFile.getPathId());
} finally {
tokenEntries.add(TokenEntry.getEOF());
}

35
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/FileAnalysisException.java
View File

@ -8,7 +8,9 @@ import java.util.Objects;
import org.apache.commons.lang3.StringUtils;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.lang.document.FileLocation;
import net.sourceforge.pmd.lang.document.PathId;
/**
@ -22,7 +24,7 @@ import net.sourceforge.pmd.lang.document.PathId;
*/
public class FileAnalysisException extends RuntimeException {
private PathId filename = PathId.UNKNOWN;
private PathId fileId = PathId.UNKNOWN;
public FileAnalysisException() {
super();
@ -40,24 +42,24 @@ public class FileAnalysisException extends RuntimeException {
super(message, cause);
}
public FileAnalysisException setFileName(PathId filename) {
this.filename = Objects.requireNonNull(filename);
public FileAnalysisException setFileId(PathId fileId) {
this.fileId = Objects.requireNonNull(fileId);
return this;
}
protected boolean hasFileName() {
return !PathId.UNKNOWN.equals(filename);
return !PathId.UNKNOWN.equals(fileId);
}
/**
* The name of the file in which the error occurred.
*/
public @NonNull PathId getFileName() {
return filename;
public @NonNull PathId getFileId() {
return fileId;
}
@Override
public String getMessage() {
public final String getMessage() {
return errorKind() + StringUtils.uncapitalize(positionToString()) + ": " + super.getMessage();
}
@ -65,11 +67,20 @@ public class FileAnalysisException extends RuntimeException {
return "Error";
}
protected String positionToString() {
protected @Nullable FileLocation location() {
return null;
}
private String positionToString() {
String result = "";
if (hasFileName()) {
return " in file '" + getFileName() + "'";
result += " in file '" + getFileId().getOriginalPath() + "'";
}
return "";
FileLocation loc = location();
if (loc != null) {
result += loc.startPosToString();
}
return result;
}
@ -85,11 +96,11 @@ public class FileAnalysisException extends RuntimeException {
*/
public static FileAnalysisException wrap(@NonNull PathId filename, @NonNull String message, @NonNull Throwable cause) {
if (cause instanceof FileAnalysisException) {
return ((FileAnalysisException) cause).setFileName(filename);
return ((FileAnalysisException) cause).setFileId(filename);
}
String fullMessage = "In file '" + filename + "': " + message;
return new FileAnalysisException(fullMessage, cause).setFileName(filename);
return new FileAnalysisException(fullMessage, cause).setFileId(filename);
}
}

37
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/ParseException.java
View File

@ -14,6 +14,7 @@ import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccTokenDocument;
import net.sourceforge.pmd.lang.document.FileLocation;
import net.sourceforge.pmd.reporting.Reportable;
import net.sourceforge.pmd.util.StringUtil;
public class ParseException extends FileAnalysisException {
@ -23,26 +24,16 @@ public class ParseException extends FileAnalysisException {
* this object has been created due to a parse error, the token
* followng this token will (therefore) be the first error token.
*/
public final @Nullable GenericToken currentToken;
public ParseException() {
super();
this.currentToken = null;
}
private @Nullable FileLocation location;
public ParseException(String message) {
super(message);
this.currentToken = null;
this.location = null;
}
public ParseException(Throwable cause) {
super(cause);
this.currentToken = null;
}
public ParseException(String message, JavaccToken token) {
super(message);
this.currentToken = token;
this.location = null;
}
/**
@ -51,14 +42,30 @@ public class ParseException extends FileAnalysisException {
public ParseException(@NonNull JavaccToken currentTokenVal,
int[][] expectedTokenSequencesVal) {
super(makeMessage(currentTokenVal, expectedTokenSequencesVal));
currentToken = currentTokenVal;
location = currentTokenVal.getNext().getReportLocation();
}
public ParseException withLocation(FileLocation loc) {
location = loc;
super.setFileId(loc.getFileId());
return this;
}
public ParseException withLocation(Reportable reportable) {
return withLocation(reportable.getReportLocation());
}
@Override
protected String errorKind() {
return "Parse exception";
}
@Override
protected @Nullable FileLocation location() {
return location;
}
/**
* It uses "currentToken" and "expectedTokenSequences" to generate a parse
* error message and returns it. If this object has been created
@ -123,8 +130,6 @@ public class ParseException extends FileAnalysisException {
if (maxSize > 1) {
retval.append(']');
}
FileLocation loc = currentToken.next.getReportLocation();
retval.append(" at ").append(loc.getStartPos().toDisplayStringInEnglish());
retval.append('.').append(eol);
if (expectedTokenSequences.length == 1) {
retval.append("Was expecting:").append(eol).append(" ");

14
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java
View File

@ -4,9 +4,11 @@
package net.sourceforge.pmd.lang.ast;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.lang.document.FileLocation;
import net.sourceforge.pmd.lang.document.PathId;
import net.sourceforge.pmd.util.StringUtil;
@ -32,7 +34,7 @@ public final class TokenMgrError extends FileAnalysisException {
this.line = line;
this.column = column;
if (filename != null) {
super.setFileName(filename);
super.setFileId(filename);
}
}
@ -55,8 +57,8 @@ public final class TokenMgrError extends FileAnalysisException {
}
@Override
protected String positionToString() {
return super.positionToString() + " at line " + line + ", column " + column;
protected @NonNull FileLocation location() {
return FileLocation.caret(getFileId(), line, column);
}
@Override
@ -67,13 +69,13 @@ public final class TokenMgrError extends FileAnalysisException {
/**
* Replace the file name of this error.
*
* @param filename New filename
* @param fileId New filename
*
* @return A new exception
*/
@Override
public TokenMgrError setFileName(PathId filename) {
super.setFileName(filename);
public TokenMgrError setFileId(PathId fileId) {
super.setFileId(fileId);
return this;
}

2
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeParserAdapter.java
View File

@ -35,7 +35,7 @@ public abstract class JjtreeParserAdapter<R extends RootNode> implements Parser
// Finally, do the parsing
return parseImpl(charStream, task);
} catch (FileAnalysisException tme) {
throw tme.setFileName(task.getTextDocument().getPathId());
throw tme.setFileId(task.getTextDocument().getPathId());
}
}

8
pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/MalformedSourceException.java
View File

@ -6,6 +6,8 @@ package net.sourceforge.pmd.lang.ast.impl.javacc;
import java.util.Objects;
import org.checkerframework.checker.nullness.qual.NonNull;
import net.sourceforge.pmd.lang.ast.FileAnalysisException;
import net.sourceforge.pmd.lang.document.FileLocation;
@ -20,12 +22,12 @@ public class MalformedSourceException extends FileAnalysisException {
public MalformedSourceException(String message, Throwable cause, FileLocation fileLocation) {
super(message, cause);
this.location = Objects.requireNonNull(fileLocation);
setFileName(fileLocation.getFileId());
setFileId(fileLocation.getFileId());
}
@Override
protected String positionToString() {
return super.positionToString() + " at " + location.startPosToString();
protected @NonNull FileLocation location() {
return location;
}
@Override

2
pmd-core/src/main/java/net/sourceforge/pmd/lang/document/CpdCompat.java
View File

@ -33,7 +33,7 @@ public final class CpdCompat {
public static TextFile cpdCompat(SourceCode sourceCode) {
return TextFile.forCharSeq(
sourceCode.getCodeBuffer(),
PathId.fromPathLikeString("fname1.dummy"),
PathId.fromPathLikeString(sourceCode.getFileName()),
dummyVersion()
);
}

6
pmd-core/src/main/java/net/sourceforge/pmd/renderers/XMLRenderer.java
View File

@ -28,7 +28,6 @@ import net.sourceforge.pmd.PMDVersion;
import net.sourceforge.pmd.Report;
import net.sourceforge.pmd.RuleViolation;
import net.sourceforge.pmd.internal.util.IOUtil;
import net.sourceforge.pmd.lang.document.PathId;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyFactory;
import net.sourceforge.pmd.util.StringUtil;
@ -188,11 +187,6 @@ public class XMLRenderer extends AbstractIncrementingRenderer {
}
}
@Override
protected String determineFileName(PathId fileId) {
return super.determineFileName(fileId);
}
@Override
public void end() throws IOException {
try {

2
pmd-core/src/test/java/net/sourceforge/pmd/processor/GlobalListenerTest.java
View File

@ -112,7 +112,7 @@ class GlobalListenerTest {
runPmd(config, listener, rule);
});
assertEquals("fname1.dummy", exception.getFileName().getOriginalPath());
assertEquals("fname1.dummy", exception.getFileId().getOriginalPath());
// cache methods are called regardless
verify(mockCache).checkValidity(any(), any(), any());

60
pmd-java/etc/grammar/Java.jjt
View File

@ -311,13 +311,7 @@ class JavaParserImpl {
}
private void throwParseException(String message) {
int line = -1;
int col = -1;
if (jj_lastpos != null) {
line = jj_lastpos.beginLine;
col = jj_lastpos.beginColumn;
}
throw new ParseException("Line " + line + ", Column " + col + ": " + message);
throw new ParseException(message).withLocation(token);
}
@ -1105,30 +1099,18 @@ void ImplementsList():
}
void PermittedSubclasses() #PermitsList:
{}
{
Token t;
}
{
t = <IDENTIFIER> {
if (!"permits".equals(t.image)) {
throw new ParseException("ERROR: expecting permits");
}
}
softKeyword("permits")
ClassOrInterfaceType()
( "," ClassOrInterfaceType() )*
}
void EnumDeclaration():
{}
{
JavaccToken t;
}
{
t = <IDENTIFIER> {
if (!"enum".equals(t.image)) {
throw new ParseException("ERROR: expecting enum");
}
}
t=<IDENTIFIER> {jjtThis.setImage(t.image);}
softKeyword("enum")
<IDENTIFIER> {setLastTokenImage(jjtThis);}
[ ImplementsList() ]
EnumBody()
}
@ -1150,16 +1132,10 @@ void EnumConstant():
}
void RecordDeclaration():
{}
{
JavaccToken t;
}
{
t = <IDENTIFIER> {
if (!"record".equals(t.image)) {
throw new ParseException("ERROR: expecting record");
}
}
t=<IDENTIFIER> {jjtThis.setImage(t.image);}
softKeyword("record")
<IDENTIFIER> {setLastTokenImage(jjtThis);}
[ TypeParameters() ]
RecordHeader()
[ ImplementsList() ]
@ -2493,16 +2469,9 @@ void CaseLabelElement(ASTSwitchLabel label) #void:
}
void Guard() #SwitchGuard:
{}
{
Token t;
}
{
t = <IDENTIFIER> {
if (!"when".equals(t.image)) {
throw new ParseException("ERROR: expected 'when'");
}
}
softKeyword("when")
Expression()
}
@ -2890,3 +2859,10 @@ void VariableAccess(): {} { <IDENTIFIER> }
void TypeExpression(): {} { <IDENTIFIER> }
void PatternExpression(): {} { <IDENTIFIER> }
void LocalClassStatement(): {} { TypeDeclaration() }
void softKeyword(String name) #void: {} {
<IDENTIFIER> {
if (!getToken(0).getImageCs().contentEquals(name))
throwParseException("Expecting keyword '" + name + "'");
}
}

2
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/internal/ReportingStrategy.java
View File

@ -52,7 +52,7 @@ public interface ReportingStrategy<T> {
@Override
public void report(Node node, String message, Void acc) {
throw new ParseException(message).setFileName(node.getTextDocument().getPathId());
throw new ParseException(message).withLocation(node);
}
};
}

5
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/JDKVersionTest.java
View File

@ -4,10 +4,11 @@
package net.sourceforge.pmd.lang.java.ast;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.List;
@ -309,7 +310,7 @@ class JDKVersionTest extends BaseJavaTreeDumpTest {
@Test
void jdk7PrivateMethodInnerClassInterface2() {
ParseException thrown = assertThrows(ParseException.class, () -> java7.parseResource("private_method_in_inner_class_interface2.java"));
assertTrue(thrown.getMessage().contains("line 19"));
assertThat(thrown.getMessage(), containsString("line 19"));
}
@Override

10
pmd-plsql/etc/grammar/PLSQL.jjt
View File

@ -719,9 +719,8 @@ ASTMethodDeclarator MethodDeclarator() :
{
throw new ParseException("FUNCTION must RETURN a value or must be WRAPPED : found \""
+ nextToken.getImage()
+ "\" at line "+nextToken.getBeginLine()
+ ", column "+nextToken.getBeginColumn()
);
+ "\""
).withLocation(nextToken);
}
}
// There is no RETURN for a WRAPPED object
@ -5428,9 +5427,8 @@ void KEYWORD(String id) #void:
{
if (!token.getImage().equalsIgnoreCase(id)) {
String eol = System.getProperty("line.separator", "\n");
throw new ParseException("Encountered \"" + token.getImage() + "\" at line "
+ token.getBeginLine() + ", column " + token.getBeginColumn() + "." + eol
+ "Was expecting: " + id);
throw new ParseException("Encountered \"" + token.getImage() + "\" "
+ "Was expecting: " + id).withLocation(token);
}
}
}

17
pmd-vm/etc/grammar/Vm.jjt
View File

@ -66,6 +66,9 @@ import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
*/
public class VmParserImpl
{
private void throwParseException(String message) {
throw new ParseException(message).withLocation(token);
}
/**
* Check whether there is a left parenthesis with leading optional
* whitespaces. This method is used in the semantic look ahead of
@ -1174,14 +1177,14 @@ VmNode Directive() :
else if (!jjtThis.isDirective())
{
// not a real directive, but maybe a Velocimacro
throw new ParseException("Invalid arg #"
+ argPos + " in VM " + t.getImage(), t);
throwParseException("Invalid arg #"
+ argPos + " in VM " + t.getImage());
}
/* if #foreach and it's the 2nd arg, ok */
else if (jjtThis.isDirective() && (!"foreach".equals(jjtThis.getDirectiveName()) || argPos != 1))
{
throw new ParseException("Invalid arg #"
+ argPos + " in directive " + t.getImage(), t);
throwParseException("Invalid arg #"
+ argPos + " in directive " + t.getImage());
}
else
{
@ -1195,9 +1198,9 @@ VmNode Directive() :
{
/* if a VM and it's the 0th arg, not ok */
throw new ParseException("Invalid first arg"
throwParseException("Invalid first arg"
+ " in #macro() directive - must be a"
+ " word token (no ' or double quote surrounding)", t);
+ " word token (no ' or double quote surrounding)");
}
}
@ -1218,7 +1221,7 @@ VmNode Directive() :
{
// VELOCITY-667 We get here if we have a "#macro" construct
// without parenthesis which is a parse error
throw new ParseException("A macro declaration requires at least a name argument", t);
throwParseException("A macro declaration requires at least a name argument");
}
/**