Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

NoViableAltException: no viable alternative at input '(' #124

Closed
2 tasks done
Chadehoc opened this issue Dec 7, 2023 · 16 comments
Closed
2 tasks done

NoViableAltException: no viable alternative at input '(' #124

Chadehoc opened this issue Dec 7, 2023 · 16 comments
Labels
bug Something isn't working

Comments

@Chadehoc
Copy link

Chadehoc commented Dec 7, 2023

Prerequisites

  • This bug is in SonarDelphi, not SonarQube or my Delphi code.
  • This bug has not already been reported.

SonarDelphi version

1.0.0

SonarQube version

10.3.0.82913

Issue description

I encountered this error, and could not isolate its cause, so this bug report is for the moment an inquiry of what to look for (to report it correctly).

The top of the stack is:

08:33:58.501 ERROR: Error during SonarScanner execution
java.lang.RuntimeException: line 139:30 no viable alternative at input '('
        at au.com.integradev.delphi.antlr.DelphiParser.reportError(DelphiParser.java:350)
        at au.com.integradev.delphi.antlr.DelphiParser.typeDecl(DelphiParser.java:6200)
        at au.com.integradev.delphi.antlr.DelphiParser.typeDeclaration(DelphiParser.java:4512)
        at au.com.integradev.delphi.antlr.DelphiParser.typeSection(DelphiParser.java:4171)
        at au.com.integradev.delphi.antlr.DelphiParser.interfaceDecl(DelphiParser.java:3379)
        at au.com.integradev.delphi.antlr.DelphiParser.unitInterface(DelphiParser.java:1688)
        at au.com.integradev.delphi.antlr.DelphiParser.unitWithoutImplementation(DelphiParser.java:1490)
        at au.com.integradev.delphi.antlr.DelphiParser.fileWithoutImplementation(DelphiParser.java:634)
        at au.com.integradev.delphi.file.DelphiFile.createAST(DelphiFile.java:171)
        at au.com.integradev.delphi.file.DelphiFile.setupFile(DelphiFile.java:129)
        at au.com.integradev.delphi.file.DelphiFile.from(DelphiFile.java:120)
        at au.com.integradev.delphi.symbol.SymbolTableBuilder.process(SymbolTableBuilder.java:282)

and the '(' token is that after class in:

unit XXX;
interface
uses
  ...;
type
  TMyRecord = record ... end;
  TMyClass = class(TParentClass<TRecordType>)

This pattern (same parent class) was processed before without problems in another project, as well as in the same project in a previously processed file (as traced with the -X option to sonar-scanner.bat).

The result is the same if I configure the projet to Delphi 11 or Delphi 12 (projects and code are different, but this piece of code is the same).

I'm juste trying out SonarQube: I followed the "Try out SonarQube" page, and sonar-delphi's README. From the trace: SonarScanner 5.0.1.3006, Java 17.0.7 Eclipse Adoptium (64-bit), Windows 10 10.0 amd64, SonarQube server 10.3.0.82913 (community).

Steps to reproduce

I'm asking for a clue to try to isolate the error. I looked at the unit processed just before this one (in the trace) and could find nothing exotic, either. Our code base is huge.

Minimal Delphi code exhibiting the issue

No response

@Chadehoc Chadehoc added bug Something isn't working triage This needs to be triaged by a maintainer labels Dec 7, 2023
@Cirras
Copy link
Collaborator

Cirras commented Dec 7, 2023

Hi @Chadehoc,

08:33:58.501 ERROR: Error during SonarScanner execution
java.lang.RuntimeException: line 139:30 no viable alternative at input '('

Right! Looks like you've run into a parsing error.

Potential causes

  • a misconfiguration that causes conditional directives to be processed incorrectly
    (or)
  • a genuine problem in the analyzer's grammar

Minimal reproducible case

Without a minimal reproducible case, this might be hard to figure out.
Let's try and isolate the line of code causing the error.

Option 1: Do it yourself

Try the following steps:

  1. Remove everything within the type definition (everything between begin and end)
    • this will confirm that the issue is somewhere within the class definition, which is my suspicion.
  2. If the parsing error is no longer present...
    • restore the code within the type definition
    • iteratively remove a chunk of code from the type definition until you identify the part that actually triggers the parsing error
    • sorry, this part is a tiny bit boring!

If the parsing error is still present after step 1, then the problem is somewhere else in the file.
This would make step 2 a lot more tedious, since you'd have to remove chunks of code from the whole file until the parsing error disappears.

Option 2: Let me do it for you

You're welcome to send the problematic file through to me:

  • at your discretion - you may want to remove the implementation section, uses clauses, and anything else that isn't required to reproduce the parsing error.
  • you can either upload it here, or email it to me at jjeleniewski@integradev.com.au

@Chadehoc
Copy link
Author

Chadehoc commented Dec 7, 2023

First reason found, a combination of:

  • Some non-ANSI identifiers. We should not use them, but there were 2 in the code base. They cause a parsing error. Good candidate for a new rule, "no accents in identifiers" (Delphi compiles, but I know that other tools handle them badly, so they should be discouraged).
  • Dependency cycle. If the former units had been processed first, I would have got a direct error, easy to understand. However, in an apparently unrelated unit, the record type was using the type having an accentuated field, and it seems enough to cause the error there.

I should be able to make a minimal example for that.

But now I have another parsing error, in another unit (but similar pattern) which resists to emptying the local types. I'm still investigating.

@zaneduffield
Copy link
Collaborator

non-ANSI identifiers

I'd be surprised if the grammar couldn't handle valid identifiers. Just a stab in the dark, have you configured the encoding for the project via sonar.sourceEncoding (default is system encoding)?

Could you give an example of such an identifier from this code?

@zaneduffield
Copy link
Collaborator

zaneduffield commented Dec 7, 2023

My apologies, I had misremembered this a bit.

I thought we handled this correctly because I looked into it a while ago and documented all the quirks in delphi unicode identifiers. We only actually fixed the whitespace issues that I discovered at that time (in #39).
The main problem with parsing right now is that only surrogate pairs of unicode codepoints are accepted, in reality delphi lets you use (almost) any codepoint above U+7F as an identifier character.

I'll raise a new issue shortly for full unicode identifier support.

@Chadehoc
Copy link
Author

Chadehoc commented Dec 8, 2023

Thanks!

Non-ANSI characters finally explain much of those parsing errors (in my case, always a french "é" or "è", as in Spécifique, in UTF-8 source code). It is tedious to track, because the location of the reported error is always elsewhere (maybe in another unit), not directly where the bad character occurs.

Now I have other exceptions thrown:

  • a MissingTokenException, missing FORWARD at 'VAR', at the VAR section of a method implementation (nothing apparently exotic). If I comment out the entire function, it passes. Should I open a new issue for that?
  • If I pass the above exception, the analysis traces two more exceptions, but they get recovered, Continuing with the next executor. Those exceptions look like:
java.lang.IllegalArgumentException: 159 is not a valid line offset for pointer. File XXX.pas has 154 character(s) at line 298
        at org.sonar.api.utils.Preconditions.checkArgument(Preconditions.java:43)                                      
        at org.sonar.api.batch.fs.internal.DefaultInputFile.checkValid(DefaultInputFile.java:374)                      
        at org.sonar.api.batch.fs.internal.DefaultInputFile.newPointer(DefaultInputFile.java:307)                      
        at org.sonar.api.batch.fs.internal.DefaultInputFile.newRange(DefaultInputFile.java:323)                        
        at au.com.integradev.delphi.reporting.DelphiIssueBuilder.createTextRange(DelphiIssueBuilder.java:228)          
        at au.com.integradev.delphi.reporting.DelphiIssueBuilder.report(DelphiIssueBuilder.java:177)                   
        at au.com.integradev.delphi.checks.TooLongLineCheck.visit(TooLongLineCheck.java:63)                            
        at au.com.integradev.delphi.checks.TooLongLineCheck.visit(TooLongLineCheck.java:31)                            
        at au.com.integradev.delphi.executor.DelphiChecksExecutor.lambda$runChecks$1(DelphiChecksExecutor.java:83)     

Should I open a new issue? Or investigate further first?

@Cirras
Copy link
Collaborator

Cirras commented Dec 8, 2023

Hello @Chadehoc,

It is tedious to track, because the location of the reported error is always elsewhere (maybe in another unit), not directly where the bad character occurs.

This is a problem in itself, I think.
I'll look into improving the error reporting when a parse fails during symbol table construction.

a MissingTokenException, missing FORWARD at 'VAR', at the VAR section of a method implementation (nothing apparently exotic). If I comment out the entire function, it passes. Should I open a new issue for that?

Yes please!
I'd appreciate it if you can wittle the function down to a minimal example case, but any code example that reproduces the parsing error would ensure I can work out the root problem.

If I pass the above exception, the analysis traces two more exceptions, but they get recovered, Continuing with the next executor. Those exceptions look like:

This is pretty interesting to me, as all known file position calculation issues were ironed out quite a long time ago.
For either of those 2 cases, is there anything particularly interesting or out-of-the-ordinary on the lines referenced by the error?

Thanks for following up with this new information, by the way.
Eager to chase these issues down for v1.1.0. 😄

@Chadehoc
Copy link
Author

Chadehoc commented Dec 8, 2023

It is tedious to track, because the location of the reported error is always elsewhere (maybe in another unit), not directly where the bad character occurs.

This is a problem in itself, I think. I'll look into improving the error reporting when a parse fails during symbol table construction.

Should I change the issue subject to "Confusing parsing error reports when non-ANSI characters in identifiers"?

Because finally, the following was another example of it:

a MissingTokenException, missing FORWARD at 'VAR', at the VAR section of a method implementation

While trimming down a minimal example, I noticed once again a é character... Here is a simple file reporting a MissingTokenException (compiles and executes ok):

program BugReport;

{$APPTYPE CONSOLE}

type
 TMyClass = class
  procedure MyFunc;
end;

procedure TMyClass.MyFunc;
begin
  // many lines here before the offending accent
  var Lé: Integer := 0;
  Writeln(Lé);
end;

procedure Main;
begin
  var X := TMyClass.Create;
  X.MyFunc;
  X.Free;
end;

begin
  Main();
  ReadLn;
end.

@Cirras
Copy link
Collaborator

Cirras commented Dec 8, 2023

Should I change the issue subject to "Confusing parsing error reports when non-ANSI characters in identifiers"?

No need, I'll raise a separate issue and just link it to this one.

While trimming down a minimal example, I noticed once again a é character...

Great! Seems like #125 is (knock on wood) going to fix all of the parsing errors, then.

That just leaves the 2 file pointer issues you mentioned.

@Chadehoc
Copy link
Author

Chadehoc commented Dec 8, 2023

That just leaves the 2 file pointer issues you mentioned.

And still the same! In the reported lines, trailing comments (after some instructions) were having accentuated characters (and these are expected in French comments!). I suppose the "exotic" characters are wrongly counted in the line length.

Fortunately, this only causes an issue when the line (including the comment) is too long.

@zaneduffield
Copy link
Collaborator

I suppose the "exotic" characters are wrongly counted in the line length.

This would make a lot of sense. Maybe one part is counting characters and the other is counting bytes.
I'll make sure to have a look at fixing that (or raising another issue) when I do #125.

@zaneduffield
Copy link
Collaborator

Fortunately, this only causes an issue when the line (including the comment) is too long.

One way to get around this issue for now would be to disable the rule entirely in a custom quality profile via the sonarqube web interface.

@zaneduffield
Copy link
Collaborator

@Chadehoc I wasn't able to reproduce the issue with the non-ASCII characters causing line offset errors in end-of-line comments.
I tried adding many accented and emoji characters to an end-of-line comment and it worked fine for me.

Would you be able to provide an example that reproduces the issue?

@fourls
Copy link
Collaborator

fourls commented Dec 12, 2023

Is it possible that the line pointer issues are due to a multiline compiler directive earlier in the file? 1.0.0 has a bug in which compiler directives that span multiple lines cause file pointer errors - it was raised in #88 and fixed earlier this month in master.

@Chadehoc
Copy link
Author

No multiline compiler directives in my case.

Here is an example line causing the mismatch (UTF-8). Splitting the comment to another line fixed it.
CreateForm(TEcranPrincipalMonoposte, LEcranPrincipal); // LEcranPrincipal doit être la 1ère vraie form créée pour être la main form fenêtre principale

@zaneduffield
Copy link
Collaborator

@Chadehoc thanks for that example, we have identified the root cause of the issue and have raised #136 to fix it.

A temporary solution would be to make sure that you are configuring your source encoding correctly.
For example with sonar.sourceEncoding=utf-8 in your sonar-project.properties file.

@Cirras Cirras removed the triage This needs to be triaged by a maintainer label Dec 13, 2023
@Cirras
Copy link
Collaborator

Cirras commented Dec 13, 2023

Looks we've identified all of the issues, then!
Glad that we have a clear idea of the root problems now.

Closing this one, and we'll focus on:

@Cirras Cirras closed this as completed Dec 13, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants