Provide a means to resolve analysis bugs in included libraries

[Scharrels]

Prerequisites

• This improvement has not already been suggested.
• This improvement would be generally useful, not specific to my code or setup.
• This improvement is not related to analysis quality (e.g. new rules, improvements to existing rules), which should be raised on the SonarDelphi repository.

Engine area

Delphi analysis

Improvement description

SonarDelphi 1.8.0 included the following change:

• Include the global library path in unit import resolution.

This change caused DelphiLint to analyze a lot more included libraries in our projects (which is a good thing, by the way).

Unfortunately, SonarDelphi crashes on one of these files, giving the following error:

In this case, I am unable to share the source code for this error, as the license for this library does not allow us to share source code. Normally, I would start my analysis of such problems with a sonar runner, enabling the -x flag to maximize debug information. In this case, a standalone Sonar runner does not show the same problem.

Our entire team is currently unable to use DelphiLint, so I still want to resolve this. Could you enhance DelphiLint to either:

• Add a possiblity to get more debug information out of a SonarDelphi run that is done within RAD Studio
• Add a possibility to ignore certain source files in the included libraries

Rationale

See other section

[Cirras]

Hi @Scharrels,

Add a possiblity to get more debug information out of a SonarDelphi run that is done within RAD Studio

This could stand to be more discoverable - but DelphiLint actually does quite a bit of logging (the equivalent of a -X).
Can you have a look at the server logs in %APPDATA%\DelphiLint\logs?

If you're able to share those logs with me (or relevant portions), I'd be happy to see what insights there are to be had.

[Scharrels]

There's not much information there, unfortunately. Here's a couple of relevant parts:

[2024-10-04 17:13:39.978 DEBUG] SonarDelphiLogOutput: [DEBUG] Execute Sensor: DelphiSensor
[2024-10-04 17:13:39.978 DEBUG] SonarDelphiLogOutput: [INFO] Platform: WINDOWS
[2024-10-04 17:13:39.978 DEBUG] SonarDelphiLogOutput: [INFO] Architecture: X64
[2024-10-04 17:13:39.978 DEBUG] SonarDelphiLogOutput: [INFO] Compiler version: 36.0
...
[2024-10-04 17:13:40.136 DEBUG] SonarDelphiLogOutput: [INFO] Conditional defines: [ASSEMBLER, DCC, CPUX64, UNICODE, DEBUG, MSWINDOWS, effects, NATIVECODE, FireDAC_SQLITE_STATIC, CONSOLE, WIN64, CPU64BITS, VER360, SvgDisableEngineHint, FRAMEWORK_VCL, CONDITIONALEXPRESSIONS]
[2024-10-04 17:13:43.443 DEBUG] SonarDelphiLogOutput: [DEBUG] Setting filesystem encoding: windows-1252
...
2024-10-04 17:13:53.668 DEBUG] SonarDelphiLogOutput: [DEBUG] 				> GridParametersWithLegend.pas
[2024-10-04 17:13:53.668 DEBUG] SonarDelphiLogOutput: [DEBUG] 					> GisUtils.pas
[2024-10-04 17:13:53.683 DEBUG] SonarDelphiLogOutput: [DEBUG] 						> GisOpenCL.pas
[2024-10-04 17:13:53.699 DEBUG] SonarDelphiLogOutput: [DEBUG] 							X GisBaseObject **Failed to locate unit**
[2024-10-04 17:13:53.746 DEBUG] SonarDelphiLogOutput: [DEBUG] 						> GisCUDA.pas
[2024-10-04 17:13:53.752 DEBUG] SonarDelphiLogOutput: [ERROR] Error while processing C:\Users\jornek\OneDrive - Gexcon AS\Documents\TatukGIS\DK11 for Delphi.RX12\Source\GisCUDA.pas
...
[2024-10-04 17:14:07.900  INFO] SonarServerUtils: 7/7 issues matched with 14 client issues and had metadata retrieved
[2024-10-04 17:14:07.902 ERROR] AnalysisServer: Error logged during SonarDelphi analysis: Error while processing C:\Users\jornek\OneDrive - Gexcon AS\Documents\TatukGIS\DK11 for Delphi.RX12\Source\GisCUDA.pas
[2024-10-04 17:14:07.902  INFO] TlvConnection: Sending ANALYZE_ERROR
[2024-10-04 17:14:07.922 DEBUG] TlvConnection: Awaiting next message

Note: This library only provides a compiled version for the missing GisBaseObject (this object handles the licensing).

[Cirras]

Hi @Scharrels,
Just to be clear, there's no accompanying stack trace with that error anywhere in the log file?
Looking at the SonarDelphi code, there should be a DelphiFileConstructionException getting logged somewhere.
https://github.com/integrated-application-development/sonar-delphi/blob/2ec51a3f6f77b75f5fd629671e26499e7d27c1ec/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/SymbolTableBuilder.java#L301-L302

[fourls]

Unfortunately there won't be a more detailed stack trace. The sonarlint-core IoC that DelphiLint uses internally does not report exceptions that occur within the IoC at all, which is really frustrating. DelphiLint attempts to report if an error occurred in a SonarDelphi analysis by injecting a custom logger that stores the most recent error-level message.

Add a possiblity to get more debug information out of a SonarDelphi run that is done within RAD Studio

I will have another look at whether we can eke more information out of the IoC because it definitely isn't ideal to have a crash, then have no idea what happened to it. We could maybe add some log redirecting support to SonarDelphi?

Add a possibility to ignore certain source files in the included libraries

Hmm, I'm not sure if that would be possible on an architectural level - if you are scanning a unit that indirectly depends on a source file, SonarDelphi will index it.

I think the biggest problem here is that at the moment DelphiLint treats any error from SonarDelphi as fatal, which is a little dramatic. SonarDelphi will happily complete a scan even if it fails to parse certain files - that could be why a standalone scan is succeeding.

Fixing this in DelphiLint would require a bit of a different approach to error management - interested to hear if anyone has any novel ideas to detect a failure. This is the information we get from the sonarlint-core IoC:

1. The issues raised by the plugin
2. The logging output from the plugin

Perhaps an analysis only counts as a failure if no issues are produced?

[fourls]

Also - @Scharrels, can you confirm whether the logs for the successful standalone scan shows a non-fatal error on the same file? That should contain a full stack trace and might give an important clue to fix the actual error.

[Scharrels]

I can confirm that a non-fatal error is shown in the stack trace of the standalone scan. The error is:

au.com.integradev.delphi.file.DelphiFile$DelphiFileConstructionException: Failed to construct DelphiFile (line 68:22 
missing SEMICOLON at 'class')

The interesting part here is that this particular part of the file is guarded by compiler directives and should have been skipped. I can't share this source file, but the following snippet should give you an idea of the code:

unit GisCUDA;

// This include file defines a lot of compiler directives, based on the target architecture. NEXTGEN is never defined here, but should be enabled manually if a user wants to include it.
{$INCLUDE SomeInclude.inc}

...

interface

{$IFDEF NEXTGEN}
type 
  SomeClass = {$IFDEF OXYGENE} public {$ENDIF}
    class ( SomeBaseClass )
    ...
  end;
{$ENDIF}

It seems that Sonar is enabling both the NEXTGEN and the OXYGENE compiler directives, while these are never defined when the program is compiled normally. I tried to disable these in sonar-project.properties:

sonar.delphi.conditionalUndefines=NEXTGEN,OXYGENE

This doesn't seem to help (it almost looks like these conditional undefines are ignored in the library path), so I'm running out of ideas here.

[fourls]

Hi @Scharrels, SonarDelphi doesn't seem to be enabling the compiler directives on a project level - in the original logs you sent, neither NEXTGEN nor OXYGENE are present in the sonar.delphi.conditionalDefines property that is aggregated from dproj files.

If they are enabled, then they must be enabled from SonarDelphi encountering a {$DEFINE OXYGENE} or {$DEFINE NEXTGEN} somewhere in the code. Are you sure this is not the case? If not, then the issue is probably not that those compiler directives are defined.

You can test if the problem is actually these directives being defined by editing the file to add something like this at the top:

{$IFDEF OXYGENE}
oxygene is defined! // will raise a parsing exception in SonarDelphi
{$ENDIF}

If, when scanned again, SonarDelphi crashes at that point instead, we know it's because OXYGENE is defined.

Is there anything else in the file that seems unusual or niche that could be causing the error? Parsing exceptions can sometimes be a little inaccurate with the exact location that the unexpected code is at.

I think the biggest problem here is that at the moment DelphiLint treats any error from SonarDelphi as fatal, which is a little dramatic. SonarDelphi will happily complete a scan even if it fails to parse certain files - that could be why a standalone scan is succeeding.

Currently working on a change to SonarDelphi error handling in DelphiLint that will note when errors occur during analysis and provide a little window for you to browse them, but won't fail the scan. I think this will be a good QoL change and will mean that you and your team will be able to start using DelphiLint again.

[Scharrels]

The behaviour that I see here is rather strange. I have modified the source file in the following manner (note: this example is simplified):

unit GisCUDA;

// This include file defines a lot of compiler directives, based on the target architecture. NEXTGEN is never defined here, but should be enabled manually if a user wants to include it.
{$INCLUDE SomeInclude.inc}

...

interface

{$IFDEF OXYGENE}
  oxygene is defined! // will raise a parsing exception in SonarDelphi
{$ENDIF}

{$IFDEF NEXTGEN}
  nextgen is defined! // will raise a parsing exception in SonarDelphi
{$ENDIF}

{$IFDEF NEXTGEN}
type 
  SomeClass = {$IFDEF OXYGENE} public {$ENDIF}
    class ( SomeBaseClass )
    ...
  end;
{$ENDIF}

What I see here is that the error is still emitted on the following line:

+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (10:35:30.536 ER...rce\GisCUDA.pas:String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError
 
au.com.integradev.delphi.file.DelphiFile$DelphiFileConstructionException: Failed to construct DelphiFile (line 94:22 
missing SEMICOLON at 'class')

This is behaviour that I didn't expect. The ifdef checks for the NEXTGEN and OXYGENE blocks are now directly above the code block that shouldn't be compiled into the source, and show that these directives are indeed disabled. The error message shows that SonarDelphi is still crashing on the block that shouldn't be executed.

[Scharrels]

I did a bit more analysis. I can trigger this exception with the following minimized file, included as a library:

unit GisCUDA;

interface

{$IFDEF OXYGENE}
  oxygene is defined! // will raise a parsing exception in SonarDelphi
{$ENDIF}

{$IFDEF NEXTGEN}
  nextgen is defined! // will raise a parsing exception in SonarDelphi
{$ENDIF}

{$IFDEF NEXTGEN}
type
  SomeType = {$IFDEF OXYGENE} public {$ENDIF} class
  end;

implementation
{$ELSE}

type
  SomeType = {$IFDEF OXYGENE} public {$ENDIF} class
  end;

implementation
{$ENDIF}

end.

I am unable to trigger it if I analyze the following unit directly (if I define it within my project):

unit UnitExample;

interface

{$IFDEF OXYGENE}
  oxygene is defined! // will raise a parsing exception in SonarDelphi
{$ENDIF}

{$IFDEF NEXTGEN}
  nextgen is defined! // will raise a parsing exception in SonarDelphi
{$ENDIF}

{$IFDEF NEXTGEN}
type
  SomeType = {$IFDEF OXYGENE} public {$ENDIF} class
  end;

implementation
{$ELSE}

type
  SomeType = {$IFDEF OXYGENE} public {$ENDIF} class
  end;

implementation
{$ENDIF}

end.

The difference here is the unit name and the fact that one of these is part of the library and the other one is included directly.

[zaneduffield]

I reduced the problem to a more minimal example

unit Bad;

interface

{$IF False}
// inside the not-taken branch
type
  Foo = {$IF False} asdf {$ENDIF} class end;
                    // ^ anything invalid
// 'implementation' must be inside the conditional branch
implementation
{$ELSE}
implementation
{$ENDIF}

end.

The above unit needs to be included into a file that is being analysed, without itself being part of the analysis. I think this has something to do with SonarDelphi's partial indexing it does for interface sections.

debug.zip can be used with either DelphiLint or a full sonar-scanner to reproduce the issue.

[fourls]

Thanks both @Scharrels and @zaneduffield, that's extremely helpful. I've raised integrated-application-development/sonar-delphi#299 to track this SonarDelphi bug - discussion can continue there if needed but I think with such a simple minimal case we should be OK.

I will keep this issue open to track the work I'm doing improving the DelphiLint experience when SonarDelphi reports errors.

[Cirras]

I've already identified the problem in SonarDelphi and will get the bug resolved for the November release.

@Scharrels, if you're interested in a stopgap measure and have the appetite for a minor patch to your 3rdParty library code, then you can simply move the implementation keyword out of these conditional branches.

So for example, this:

{$IF ABC}
  // ...
  implementation
{$ELSE}
  // ...
  implementation
{$ENDIF}

Would become this:

{$IF ABC}
  // ...
{$ELSE}
  // ...
{$ENDIF}

implementation

[Scharrels]

I'm glad that the bug has been identified. Thank you all for the time invested in this bug, and in this project in general :)

[fourls]

@Scharrels, the change stopping DelphiLint from falling over on non-fatal errors has been merged in #67. If you'd like the changes you can build a new version from latest master or wait until the next release (which will be soon).