Page MenuHomePhabricator

ArcanistCSharpLinter not reporting linter warnings
Closed, WontfixPublic

Description

It seems that arcanist is not handling/reporting the warnings produced by cslint. When I run the exact same command as arc lint runs directly on cslint it produces a lot of warnings, but arc lint replies with no warnings.

Could somebody please confirm that the ArcanistCSharpLinter is reporting lint warnings on windows (or linux for that matter)?

I am running this on windows with the following arcanist configs:
In .arcconfig: "lint.engine" : "ArcanistConfigurationDrivenLintEngine"
In .arclint:

{
  "linters": {
    "csharp": {
      "type": "csharp",
      "include": "(\\.cs$)",
      "binary": "c:/_CODE_/github/cstools/cslint/bin/Debug/cslint.exe",
      "discovery": {
        "([^\\\\]+)\\\\(.*?)\\.cs": [
          "$1\\$1.csproj"
        ]
      }
    }
  }
}

Trace of arc lint and the seperate cslint command:

C:\_CODE_\PhabGit\testrepo>arc lint --trace
libphutil loaded from 'C:\arc\libphutil\src'.
arcanist loaded from 'C:\arc\arcanist\src'.
Working Copy: Reading .arcconfig from "C:\_CODE_\PhabGit\testrepo/.arcconfig".
Working Copy: Path "C:\_CODE_\PhabGit\testrepo" is part of `git` working copy "C
:\_CODE_\PhabGit\testrepo".
Working Copy: Project root is at "C:\_CODE_\PhabGit\testrepo".
>>> [0] <exec> $ git rev-parse --verify HEAD^
<<< [0] <exec> 39,001 us
>>> [1] <exec> $ git rev-parse --abbrev-ref --symbolic-full-name '@{upstream}'
<<< [1] <exec> 38,002 us
>>> [2] <exec> $ git rev-parse --git-dir
<<< [2] <exec> 36,001 us
>>> [3] <exec> $ git cat-file -t "origin/master"
<<< [3] <exec> 40,002 us
>>> [4] <exec> $ git merge-base "origin/master" HEAD
<<< [4] <exec> 43,002 us
>>> [5] <exec> $ git diff --no-ext-diff --no-textconv --raw "b5f3fa18fb02d155a85
228074a8eabf321b9c294" --
<<< [5] <exec> 43,001 us
>>> [6] <exec> $ git diff --no-ext-diff --no-textconv --raw "HEAD" --
>>> [7] <exec> $ git ls-files --others --exclude-standard
>>> [8] <exec> $ git diff-files --name-only
<<< [7] <exec> 61,002 us
<<< [6] <exec> 76,004 us
<<< [8] <exec> 62,003 us
>>> [9] <exec> $ git diff --no-ext-diff --no-textconv --no-color --src-prefix=a/
 --dst-prefix=b/ -U32767 -M -C "b5f3fa18fb02d155a85228074a8eabf321b9c294" --
<<< [9] <exec> 46,002 us
>>> [10] <lint> ArcanistCSharpLinter <paths = 2>
>>> [11] <exec> $ C:\_CODE_\github\cstools\cslint\bin\Debug\cslint.exe -v
<<< [11] <exec> 230,012 us
<<< [10] <lint> 231,013 us
>>> [12] <lint> ArcanistCSharpLinter
>>> [13] <exec> $ C:\_CODE_\github\cstools\cslint\bin\Debug\cslint.exe --setting
s-base64="eyIoW15cXFxcXSspXFxcXCguKj8pXFwuY3MiOlsiJDFcXCQxLmNzcHJvaiJdfQ==" -r=.
 "C:\_CODE_\PhabGit\testrepo\MyTestSolution\MyTestSolution\App.xaml.cs" "C:\_COD
E_\PhabGit\testrepo\MyTestSolution\MyTestSolution\Class1.cs"
<<< [13] <exec> 1,915,109 us
<<< [12] <lint> 1,917,109 us
 OKAY  No lint warnings.

C:\_CODE_\PhabGit\testrepo>C:\_CODE_\github\cstools\cslint\bin\Debug\cslint.exe
--settings-base64="eyIoW15cXFxcXSspXFxcXCguKj8pXFwuY3MiOlsiJDFcXCQxLmNzcHJvaiJdf
Q==" -r=. "C:\_CODE_\PhabGit\testrepo\MyTestSolution\MyTestSolution\App.xaml.cs"
 "C:\_CODE_\PhabGit\testrepo\MyTestSolution\MyTestSolution\Class1.cs"
[{"Issues":[{"LineNumber":13,"Column":5,"Index":{"Name":"Class Name Does Not Mat
ch Filename","Code":"CS0003","Message":"The only public class in a file should m
atch the name of the file that it is declared in.  The class name was '%s' but t
he filename is '%s'.","Severity":2},"Parameters":["App","App.xaml"],"Replacement
Text":null,"OriginalText":null}],"FileName":"MyTestSolution\\MyTestSolution\\App
.xaml.cs","BaseName":"App.xaml.cs"},{"Issues":[{"LineNumber":17,"Column":5,"Inde
x":{"Name":"Class Name Does Not Match Filename","Code":"CS0003","Message":"The o
nly public class in a file should match the name of the file that it is declared
 in.  The class name was '%s' but the filename is '%s'.","Severity":2},"Paramete
rs":["Class15","Class1"],"ReplacementText":null,"OriginalText":null},{"LineNumbe
r":22,"Column":13,"Index":{"Name":"Use 'var' For Implicit Variable Declarations"
,"Code":"CS0004","Message":"'var' should be used when declaring initialized vari
ables as it helps keep code clean and readable.","Severity":1},"Parameters":[],"
ReplacementText":"var","OriginalText":"string"},{"LineNumber":31,"Column":13,"In
dex":{"Name":"Use 'var' For Implicit Variable Declarations","Code":"CS0004","Mes
sage":"'var' should be used when declaring initialized variables as it helps kee
p code clean and readable.","Severity":1},"Parameters":[],"ReplacementText":"var
","OriginalText":"int"}],"FileName":"MyTestSolution\\MyTestSolution\\Class1.cs",
"BaseName":"Class1.cs"}]
C:\_CODE_\PhabGit\testrepo>C:\_CODE_\github\cstools\cslint\bin\Debug\cslint.exe
-v
1

Event Timeline

adriaan.dehaan assigned this task to hach-que.
adriaan.dehaan raised the priority of this task from to Needs Triage.
adriaan.dehaan updated the task description. (Show Details)
adriaan.dehaan added a project: Arcanist.

I had the same problem and tracked it down to the following facts:

  • Under Windows cslint.exe produces errors/warnings using \ as path delimiter
  • arc lint checks for lines changed by the committer by parsing the output of hg diff / git diff. These diffs use / as the path delimiter
  • arc lint uses the / variants as keys to filter out warnings on lines, which have not been changed in the current diff. Those keys don't match the output of cslint.exe's \ variants

This leads to all warnings being filtered out.

I fixed this by modifying the parsing of diff outputs. (arcanist/src/parser/ArcanistDiffParser.php)

...

if (isset($match['type'])) {
  if ($match['type'] == 'diff --git') {
    list($old, $new) = self::splitGitDiffPaths($match['oldnew']);

    // Handle back slash path delimiter when run under windows
    if (phutil_is_windows()) {
      $old = str_replace('/', '\\', $old);
      $new = str_replace('/', '\\', $new);
    }

    $match['old'] = $old;
    $match['cur'] = $new;
  }
}

...

This does work for me at the moment, but I can't say if this quick fix will play together well with other linters (I am using the C# linter exclusively)

hach-que removed hach-que as the assignee of this task.

cslint is deprecated, see T8978 for more information.