Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

column offset treated as character offset for line number 1 pinned sarif report where line is too short to contain offset #536

Open
jsoref opened this issue Oct 26, 2023 · 0 comments

Comments

@jsoref
Copy link

jsoref commented Oct 26, 2023

version info

Version: 1.83.1 (Universal)
Commit: f1b07bd25dfad64b0167beb15359ae573aecd2cc
Date: 2023-10-10T23:46:55.789Z
Electron: 25.8.4
ElectronBuildId: 24154031
Chromium: 114.0.5735.289
Node.js: 18.15.0
V8: 11.4.183.29-electron.0
OS: Darwin arm64 23.0.0

MS-SarifVSCode.sarif-viewer v3.4.2

Note that the selection is on line 4:

image

GitHub's renderer just assigns the report to line 1 without making a selection:

https://github.com/check-spelling-sandbox/fsharp/security/code-scanning/23230
image

  1. The file path is:
    tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/MemberDefinitions/NamedArguments/PropSetAfterConstrn02NamedExt.fs

  2. The text being flagged is Constrn which is at offset ~115

  3. Line one is // #Regression #Conformance #DeclarationElements #MemberDefinitions #NamedArguments which is ~84 characters long

  4. This add-on appears to be counting characters and has found that 115 characters is:

    // #Regression #Conformance #DeclarationElements #MemberDefinitions #NamedArguments 
    #light
    
    // FSB 1368, named arg
    

    that results in uments being highlighted.

I'm sure there's no "correct" way to interpret the somewhat garbage input being provided (it's nice to be able to hang reports about file paths in sarif reports, but the reporting mechanism doesn't seem to offer a real way to do it, so I'm abusing line 1 which usually exists and works well enough in GitHub's visualization), but I'm personally much happier with the way that GitHub's thing handles it than the way this.

There is currently a sarif artifact available if people want to ingest this:
https://github.com/check-spelling-sandbox/fsharp/suites/17651709919/artifacts/1009381330

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant