Improve reporting of preprocessor-related parsing failures

Line information was incorrect or incomplete in some cases:
- within include files
- within compiler directives
- when include processing fails due to self-referencing include files

This was discovered due to the new `ParsingError` check triggering file
pointer errors when trying to raise issues on included tokens.

Author: cirras

CHANGELOG.md

@@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Exclude types annotated with attributes in `UnusedType`.
+- Improve reporting of parsing failures occurring within include files.
+- Improve reporting of parsing failures occurring within compiler directives.
 
 ### Deprecated
 

delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g

@@ -100,6 +100,9 @@ tokens {
 package au.com.integradev.delphi.antlr;
 
 import au.com.integradev.delphi.antlr.ast.node.*;
+import au.com.integradev.delphi.antlr.ast.token.DelphiTokenImpl;
+import au.com.integradev.delphi.antlr.ast.token.IncludeToken;
+import au.com.integradev.delphi.utils.LocatableException;
 }
 
 @lexer::header
@@ -127,6 +130,7 @@ import au.com.integradev.delphi.antlr.ast.node.*;
 package au.com.integradev.delphi.antlr;
 
 import org.apache.commons.lang3.StringUtils;
+import au.com.integradev.delphi.utils.LocatableException;
 }
 
 @lexer::members {
@@ -143,15 +147,23 @@ import org.apache.commons.lang3.StringUtils;
   public void reportError(RecognitionException e) {
     String hdr = this.getErrorHeader(e);
     String msg = this.getErrorMessage(e, this.getTokenNames());
-    throw new LexerException(hdr + " " + msg, e.line, e);
+    throwLexerException(hdr + " " + msg, e.line, e);
   }
 
   @Override
   public String getErrorHeader(RecognitionException e) {
     return "line " + e.line + ":" + e.charPositionInLine;
   }
 
-  public static class LexerException extends RuntimeException {
+  private void throwLexerException(String message, int line) {
+    throwLexerException(message, line, null);
+  }
+
+  protected void throwLexerException(String message, int line, Throwable cause) {
+    throw new LexerException(message, line, cause);
+  }
+
+  public static class LexerException extends RuntimeException implements LocatableException {
     private final int line;
 
     public LexerException(String message, int line) {
@@ -164,6 +176,7 @@ import org.apache.commons.lang3.StringUtils;
       this.line = line;
     }
 
+    @Override
     public int getLine() {
       return line;
     }
@@ -237,13 +250,14 @@ import org.apache.commons.lang3.StringUtils;
           break;
 
         case EOF:
-          throw new LexerException(
+          throwLexerException(
               "line "
                   + state.tokenStartLine
                   + ":"
                   + state.tokenStartCharPositionInLine
                   + " unterminated multi-line comment",
               state.tokenStartLine);
+          break;
 
         default:
           // do nothing
@@ -334,22 +348,29 @@ import org.apache.commons.lang3.StringUtils;
   }
 
   private Token changeTokenType(int type, int offset) {
-    CommonToken t = new CommonToken(input.LT(offset));
-    t.setType(type);
-    return t;
+    CommonToken result = cloneToken((CommonToken) input.LT(offset));
+    result.setType(type);
+    return result;
   }
 
   private Token combineLastNTokens(int type, int count) {
     CommonToken firstToken = (CommonToken) input.LT(-count);
     CommonToken lastToken = (CommonToken) input.LT(-1);
-    CommonToken result = new CommonToken(lastToken);
+    CommonToken result = cloneToken(lastToken);
     result.setType(type);
     result.setStartIndex(firstToken.getStartIndex());
     result.setLine(firstToken.getLine());
     result.setCharPositionInLine(firstToken.getCharPositionInLine());
     return result;
   }
 
+  private static CommonToken cloneToken(CommonToken other) {
+    if (other instanceof IncludeToken) {
+      return new IncludeToken((IncludeToken) other);
+    }
+    return new CommonToken(other);
+  }
+
   private BinaryExpressionNodeImpl createBinaryExpression(Object operator) {
     Token token = adaptor.getToken(operator);
     return new BinaryExpressionNodeImpl(token);
@@ -359,7 +380,20 @@ import org.apache.commons.lang3.StringUtils;
   public void reportError(RecognitionException e) {
     String hdr = this.getErrorHeader(e);
     String msg = this.getErrorMessage(e, this.getTokenNames());
-    throw new ParserException(hdr + " " + msg, e.line, e);
+
+    String message = hdr + " " + msg;
+    int line;
+
+    if (e.token == null) {
+      line = 0;
+    } else if (e.token instanceof IncludeToken) {
+      line = ((DelphiTokenImpl) ((IncludeToken) e.token).getInsertionToken()).getBeginLine();
+      message = "included on line " + line + " :: " + message;
+    } else {
+      line = e.token.getLine();
+    }
+
+    throw new ParserException(message, line, e);
   }
 
   @Override
@@ -381,16 +415,17 @@ import org.apache.commons.lang3.StringUtils;
     resetBinaryExpressionTokens(binaryExpression.getRight());
   }
 
-  public static class ParserException extends RuntimeException {
+  public static class ParserException extends RuntimeException implements LocatableException {
     private final int line;
 
     public ParserException(String message, int line, Throwable cause) {
       super(message, cause);
       this.line = line;
     }
 
+    @Override
     public int getLine() {
-      return line;
+      return this.line;
     }
   }
 }

delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/token/DelphiTokenImpl.java

@@ -84,11 +84,11 @@ public int getEndColumn() {
 
   private void calculatePosition() {
     if (isIncludedToken()) {
-      FilePosition insertionPosition = ((IncludeToken) token).getInsertionPosition();
-      beginLine = insertionPosition.getBeginLine();
-      beginColumn = insertionPosition.getBeginColumn();
-      endLine = insertionPosition.getEndLine();
-      endColumn = insertionPosition.getEndColumn();
+      DelphiToken insertionToken = ((IncludeToken) token).getInsertionToken();
+      beginLine = insertionToken.getBeginLine();
+      beginColumn = insertionToken.getBeginColumn();
+      endLine = insertionToken.getEndLine();
+      endColumn = insertionToken.getEndColumn();
     } else if (isComment() || isCompilerDirective()) {
       TokenLocation location =
           new TokenLocation(token.getLine(), token.getCharPositionInLine(), token.getText());

delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/token/IncludeToken.java

@@ -18,20 +18,31 @@
  */
 package au.com.integradev.delphi.antlr.ast.token;
 
+import org.antlr.runtime.CharStream;
 import org.antlr.runtime.CommonToken;
 import org.antlr.runtime.Token;
-import org.sonar.plugins.communitydelphi.api.check.FilePosition;
 import org.sonar.plugins.communitydelphi.api.token.DelphiToken;
 
 public class IncludeToken extends CommonToken {
-  private final FilePosition insertionPosition;
+  private final DelphiToken insertionToken;
+
+  public IncludeToken(
+      CharStream input, int type, int channel, int start, int stop, DelphiToken insertionToken) {
+    super(input, type, channel, start, stop);
+    this.insertionToken = insertionToken;
+  }
 
   public IncludeToken(Token token, DelphiToken insertionToken) {
     super(token);
-    this.insertionPosition = FilePosition.from(insertionToken);
+    this.insertionToken = insertionToken;
+  }
+
+  public IncludeToken(IncludeToken other) {
+    super(other);
+    this.insertionToken = other.insertionToken;
   }
 
-  public FilePosition getInsertionPosition() {
-    return insertionPosition;
+  public DelphiToken getInsertionToken() {
+    return insertionToken;
   }
 }

delphi-frontend/src/main/java/au/com/integradev/delphi/file/DelphiFile.java

@@ -28,6 +28,7 @@
 import au.com.integradev.delphi.preprocessor.CompilerSwitchRegistry;
 import au.com.integradev.delphi.preprocessor.DelphiPreprocessor;
 import au.com.integradev.delphi.preprocessor.DelphiPreprocessorFactory;
+import au.com.integradev.delphi.preprocessor.PreprocessorException;
 import au.com.integradev.delphi.preprocessor.TextBlockLineEndingModeRegistry;
 import au.com.integradev.delphi.preprocessor.search.SearchPath;
 import au.com.integradev.delphi.utils.DelphiUtils;
@@ -157,6 +158,7 @@ private static void setupFile(
         | RecognitionException
         | LexerException
         | ParserException
+        | PreprocessorException
         | EmptyDelphiFileException e) {
       throw new DelphiFileConstructionException(e);
     }

delphi-frontend/src/main/java/au/com/integradev/delphi/preprocessor/DelphiPreprocessor.java

@@ -35,6 +35,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Iterables;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
@@ -46,6 +47,7 @@
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.stream.Collectors;
+import org.antlr.runtime.CharStream;
 import org.antlr.runtime.Token;
 import org.apache.commons.io.FilenameUtils;
 import org.slf4j.Logger;
@@ -229,43 +231,46 @@ private List<Token> processIncludeFile(String filename, Path includePath, Delphi
       Path includeFile = config.getSearchPath().search(filename, includePath);
 
       if (includeFile != null) {
-        String path = includeFile.toAbsolutePath().normalize().toString();
-
-        if (path.equals(lexer.getSourceName())) {
+        Path lexerFile = Path.of(lexer.getSourceName());
+        if (Files.isSameFile(includeFile, lexerFile)) {
           throw new SelfReferencingIncludeFileException(
-              "Include file <" + includeFile.toAbsolutePath() + "> references itself");
+              String.format("Include file '%s' references itself", includeFile.toAbsolutePath()),
+              location);
         }
 
-        DelphiFileStream fileStream = new DelphiFileStream(path, config.getEncoding());
-        DelphiLexer includeLexer = new DelphiLexer(fileStream);
-        DelphiPreprocessor preprocessor =
-            new DelphiPreprocessor(
-                includeLexer,
-                config,
-                platform,
-                definitions,
-                currentSwitches,
-                switchRegistry,
-                textBlockLineEndingModeRegistry,
-                location.getIndex(),
-                true);
-
-        preprocessor.process();
-
-        List<Token> includeTokens = preprocessor.getTokenStream().getTokens();
-        return includeTokens.stream()
-            .limit(includeTokens.size() - 1L)
-            .map(token -> new IncludeToken(token, location))
-            .collect(Collectors.toList());
+        List<Token> result = preprocessIncludeFile(location, includeFile);
+        result.remove(result.size() - 1); // Remove EOF token
+        return result;
       }
-    } catch (IOException | RuntimeException e) {
+    } catch (IOException e) {
       LOG.debug("Error occurred while resolving includes: ", e);
     }
 
     LOG.warn("Failed to resolve include '{}'.", filename);
     return Collections.emptyList();
   }
 
+  private List<Token> preprocessIncludeFile(DelphiToken location, Path path) throws IOException {
+    var fileStream = new DelphiFileStream(path.toAbsolutePath().toString(), config.getEncoding());
+    DelphiLexer includeLexer = new DelphiIncludeLexer(fileStream, location);
+
+    DelphiPreprocessor preprocessor =
+        new DelphiPreprocessor(
+            includeLexer,
+            config,
+            platform,
+            definitions,
+            currentSwitches,
+            switchRegistry,
+            textBlockLineEndingModeRegistry,
+            location.getIndex(),
+            true);
+
+    preprocessor.process();
+
+    return preprocessor.getTokenStream().getTokens();
+  }
+
   private void addBranchingDirective(BranchingDirective directive) {
     if (!parentDirective.isEmpty()) {
       parentDirective.peek().addDirective(directive);
@@ -363,9 +368,43 @@ public List<DelphiToken> getRawTokens() {
         .collect(Collectors.toUnmodifiableList());
   }
 
-  static class SelfReferencingIncludeFileException extends RuntimeException {
-    SelfReferencingIncludeFileException(String message) {
-      super(message);
+  static class SelfReferencingIncludeFileException extends PreprocessorException {
+    SelfReferencingIncludeFileException(String message, DelphiToken token) {
+      super(message, token);
+    }
+  }
+
+  private static class DelphiIncludeLexer extends DelphiLexer {
+    private final DelphiToken insertionToken;
+
+    public DelphiIncludeLexer(CharStream input, DelphiToken insertionToken) {
+      super(input);
+      this.insertionToken = insertionToken;
+    }
+
+    @Override
+    public Token emit() {
+      Token token =
+          new IncludeToken(
+              input,
+              state.type,
+              state.channel,
+              state.tokenStartCharIndex,
+              getCharIndex() - 1,
+              insertionToken);
+      token.setLine(state.tokenStartLine);
+      token.setText(state.text);
+      token.setCharPositionInLine(state.tokenStartCharPositionInLine);
+      emit(token);
+      return token;
+    }
+
+    @Override
+    protected void throwLexerException(String message, int line, Throwable cause) {
+      super.throwLexerException(
+          String.format("included on line %d :: %s", insertionToken.getBeginLine(), message),
+          insertionToken.getBeginLine(),
+          cause);
     }
   }
 }