integrated-application-development / sonar-delphi

Delphi language plugin for SonarQube
GNU Lesser General Public License v3.0
104 stars 17 forks source link

File pointer errors on the first line of files where BOM is present #234

Closed Daniel-Tr closed 6 months ago

Daniel-Tr commented 6 months ago

Prerequisites

SonarDelphi version

1.5.0

SonarQube version

10.5.1

Issue description

When making a complete unit deprecated with a short justification, the first line of the unit might grow over 120 chars quite fast. SonarDelphi fails in this situation though with an error message that some index is not a valid index.

Steps to reproduce

  1. Create a new unit 'MyDeprecatedTestUnit.pas' in a project
  2. Declare the unit as deprecated with a justification/steps to resolve deprecated warnings:
unit MyDeprecatedTestUnit deprecated 'The mechanism in this unit has been replaced with another one, see MyNewApproach.ps -oSomeUser -c05/15/2024';
interface
const
implementation
end.
  1. Note that the project compiles, showing a deprecated warning if the unit is referenced
  2. Ensure the "Lines should not be too long" rule is active and configured to a limit of 120 (seems both to be default)
  3. Analyze the project
  4. Exception during the parsing

Minimal Delphi code exhibiting the issue

No response

zaneduffield commented 6 months ago

@Daniel-Tr thanks for the report! Would you be able to provide a stacktrace for this exception? I wasn't able to reproduce it, but I suspect it's related to non-ASCII data and the LineTooLongRule.

It might be because the correct encoding for the file is not configured (via sonar.sourceEncoding=<name> in sonar-project.properties - see the manual), but it could also be a bug in the implementation.

If it's not an encoding problem, would you be able to provide another example of a line that reproduces the issue?

Daniel-Tr commented 6 months ago

Sure. I just noticed that this would only occur when the "Lines should not be too long" rule is active. I adjusted the repro steps accordingly.

It happens for pretty much every first line that is long enough for me. I just added A's: MyDeprecatedTestUnit.zip

The file is UTF-8-formatted and the source code encoding set to UTF-8:

INFO: SonarScanner 5.0.1.3006 INFO: Java 17.0.7 Eclipse Adoptium (64-bit) INFO: Analyzing on SonarQube server 10.5.1.90531 INFO: Default locale: "de_DE", source code encoding: "UTF-8"

ERROR: Error occurred while running check TooLongLineCheck on file: MyDeprecatedTestUnit.pas java.lang.IllegalArgumentException: 148 is not a valid line offset for pointer. File DelphiLib/MyDeprecatedTestUnit.pas has 147 character(s) at line 1 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.DelphiIssueBuilderImpl.createTextRange(DelphiIssueBuilderImpl.java:332) at au.com.integradev.delphi.reporting.DelphiIssueBuilderImpl.report(DelphiIssueBuilderImpl.java:221) 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:89) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(Unknown Source) at java.base/java.util.stream.ReferencePipeline$2$1.accept(Unknown Source) at java.base/java.util.HashMap$KeySpliterator.forEachRemaining(Unknown Source) at java.base/java.util.stream.AbstractPipeline.copyInto(Unknown Source) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source) at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(Unknown Source) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(Unknown Source) at java.base/java.util.stream.AbstractPipeline.evaluate(Unknown Source) at java.base/java.util.stream.ReferencePipeline.forEach(Unknown Source) at au.com.integradev.delphi.executor.DelphiChecksExecutor.runChecks(DelphiChecksExecutor.java:84) at au.com.integradev.delphi.executor.DelphiChecksExecutor.execute(DelphiChecksExecutor.java:71) at au.com.integradev.delphi.executor.DelphiMasterExecutor.executeExecutor(DelphiMasterExecutor.java:74) at au.com.integradev.delphi.executor.DelphiMasterExecutor.execute(DelphiMasterExecutor.java:52) at au.com.integradev.delphi.DelphiSensor.executeOnFiles(DelphiSensor.java:144) at au.com.integradev.delphi.DelphiSensor.execute(DelphiSensor.java:83) at org.sonar.scanner.sensor.AbstractSensorWrapper.analyse(AbstractSensorWrapper.java:64) at org.sonar.scanner.sensor.ModuleSensorsExecutor.execute(ModuleSensorsExecutor.java:88) at org.sonar.scanner.sensor.ModuleSensorsExecutor.lambda$execute$1(ModuleSensorsExecutor.java:61) at org.sonar.scanner.sensor.ModuleSensorsExecutor.withModuleStrategy(ModuleSensorsExecutor.java:79) at org.sonar.scanner.sensor.ModuleSensorsExecutor.execute(ModuleSensorsExecutor.java:61) at org.sonar.scanner.scan.SpringModuleScanContainer.doAfterStart(SpringModuleScanContainer.java:82) at org.sonar.core.platform.SpringComponentContainer.startComponents(SpringComponentContainer.java:226) at org.sonar.core.platform.SpringComponentContainer.execute(SpringComponentContainer.java:205) at org.sonar.scanner.scan.SpringProjectScanContainer.scan(SpringProjectScanContainer.java:204) at org.sonar.scanner.scan.SpringProjectScanContainer.scanRecursively(SpringProjectScanContainer.java:200) at org.sonar.scanner.scan.SpringProjectScanContainer.doAfterStart(SpringProjectScanContainer.java:173) at org.sonar.core.platform.SpringComponentContainer.startComponents(SpringComponentContainer.java:226) at org.sonar.core.platform.SpringComponentContainer.execute(SpringComponentContainer.java:205) at org.sonar.scanner.bootstrap.SpringScannerContainer.doAfterStart(SpringScannerContainer.java:351) at org.sonar.core.platform.SpringComponentContainer.startComponents(SpringComponentContainer.java:226) at org.sonar.core.platform.SpringComponentContainer.execute(SpringComponentContainer.java:205) at org.sonar.scanner.bootstrap.SpringGlobalContainer.doAfterStart(SpringGlobalContainer.java:138) at org.sonar.core.platform.SpringComponentContainer.startComponents(SpringComponentContainer.java:226) at org.sonar.core.platform.SpringComponentContainer.execute(SpringComponentContainer.java:205) at org.sonar.batch.bootstrapper.Batch.doExecute(Batch.java:71) at org.sonar.batch.bootstrapper.Batch.execute(Batch.java:65) at org.sonarsource.scanner.api.internal.batch.BatchIsolatedLauncher.execute(BatchIsolatedLauncher.java:46) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.base/java.lang.reflect.Method.invoke(Unknown Source) at org.sonarsource.scanner.api.internal.IsolatedLauncherProxy.invoke(IsolatedLauncherProxy.java:60) at jdk.proxy1/jdk.proxy1.$Proxy0.execute(Unknown Source) at org.sonarsource.scanner.api.EmbeddedScanner.doExecute(EmbeddedScanner.java:189) at org.sonarsource.scanner.api.EmbeddedScanner.execute(EmbeddedScanner.java:138) at org.sonarsource.scanner.cli.Main.execute(Main.java:126) at org.sonarsource.scanner.cli.Main.execute(Main.java:81) at org.sonarsource.scanner.cli.Main.main(Main.java:62)

Cirras commented 6 months ago

Thanks for those additional details, super helpful. 👍

I have a hunch that this is caused by the UTF-8 BOM erroneously appearing in the string data for the first line and offsetting the text pointer by 1 character.

I'll have a look to confirm that soon.

Cirras commented 6 months ago

I've confirmed the problem. Looks like it is indeed a BOM issue, and should be easy to fix.

When constructing a DelphiFile, we read the file in 2 different ways (DelphiFileStream and FileUtils.readLines. DelphiFileStream handles the BOM and removes it, FileUtils.readLines persists the BOM into the string data and causes the mismatch that we're seeing here.

It should be as easy as unifying our file-reading approach. We already have to read the file into a char array in DelphiFileStream, so we can just grab the string data from there and split it into lines ourselves.

I'll have a look at doing this shortly.