Page MenuHomePhabricator
Create Task
Maniphest T10237

Pylint Linter fails on some specific python cases
Closed, DuplicatePublic
Actions

Description

Somewhere in my code, I had a regex with four | operator. At this point pylint linter failed because it was assuming that line as a valid pylint error message (Sometimes pylint shows code in error messages).

Relevant code in ArcanistPyLintLinter:

foreach ($lines as $line) {
    $matches = explode('|', $line, 5);

    if (count($matches) < 5) { 
      continue;
    }

    // NOTE: PyLint sometimes returns -1 as the character offset for a
    // message. If it does, treat it as 0. See T9257.
    $char = (int)$matches[1];
    $char = max(0, $char);

    $message = id(new ArcanistLintMessage())
      ->setPath($path)
      ->setLine($matches[0])
      ->setChar($char)
      ->setCode($matches[2])
      ->setSeverity($this->getLintMessageSeverity($matches[2]))
      ->setName(ucwords(str_replace('-', ' ', $matches[3])))
      ->setDescription($matches[4]);

    $messages[] = $message;
  }

Here if (count($matches) < 5) { is assuming that if a pylint message has atleast four | then it is valid pylint message.

Here is the error message trace:

[2016-01-28 21:44:37] EXCEPTION: (PhutilAggregateException) Some linters failed:
    - Exception: Parameter passed to "setLine()" must be an integer. at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:274]
arcanist(head=master, ref.master=4f1141d0c59b), libarcdjango(head=remove_print, ref.master=d2545efd4127, ref.remove_print=d64683a74983), phutil(head=master, ref.master=f0881b37049c)
  #0 <#2> ArcanistLintMessage::validateInteger(string, string) called at [<arcanist>/src/lint/ArcanistLintMessage.php:75]
  #1 <#2> ArcanistLintMessage::setLine(string) called at [<arcanist>/src/lint/linter/ArcanistPyLintLinter.php:139]
  #2 <#2> ArcanistPyLintLinter::parseLinterOutput(string, integer, string, string) called at [<arcanist>/src/lint/linter/ArcanistExternalLinter.php:437]
  #3 <#2> ArcanistExternalLinter::resolveFuture(string, ExecFuture) called at [<arcanist>/src/lint/linter/ArcanistFutureLinter.php:34]
  #4 <#2> ArcanistFutureLinter::didLintPaths(array) called at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:594]
  #5 <#2> ArcanistLintEngine::executeDidLintOnPaths(ArcanistPyLintLinter, array) called at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:545]
  #6 <#2> ArcanistLintEngine::executeLintersOnChunk(array, array) called at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:473]
  #7 <#2> ArcanistLintEngine::executeLinters(array) called at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:216]
  #8 ArcanistLintEngine::run() called at [<arcanist>/src/workflow/ArcanistLintWorkflow.php:334]
  #9 ArcanistLintWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]
<<< [58] <exec> 14,957,439 us
<<< [88] <exec> 8,930,099 us
<<< [89] <exec> 7,174,690 us

Related Objects

Event Timeline

DheerendraRathor created this task.Jan 28 2016, 9:50 PM
DheerendraRathor added a comment.EditedJan 28 2016, 10:04 PM

For now I've modified my local copy of this file and did few changes like:

protected function getMandatoryFlags() {
    $options = array();

    $options[] = '--reports=no';
    $options[] = '--msg-template=#ArcanistPyLintLinter:{line}|{column}|{msg_id}|{symbol}|{msg}';  // Modified to customize pylint message so that linter won't fail on specific python cases

    // Specify an `--rcfile`, either absolute or relative to the project root.
    // Stupidly, the command line args above are overridden by rcfile, so be
    // careful.
    $config = $this->config;
    if ($config !== null) {
      $options[] = '--rcfile='.$config;
    }

    return $options;
  }
foreach ($lines as $line) {
      if (!(substr( $line, 0, 22 ) === "#ArcanistPyLintLinter:")){
        continue;
      }

      $line = str_replace("#ArcanistPyLintLinter:", "", $line);

      $matches = explode('|', $line, 5);

      if (count($matches) < 5) {
        continue;
      }
     //Remaining code
DheerendraRathor updated the task description. (Show Details)Jan 28 2016, 10:04 PM
avivey added a project: Lint.Jan 28 2016, 10:21 PM
partofthething added a subscriber: partofthething.Jun 18 2016, 12:17 AM

I just encountered the same problem and this solution worked for me as well. Thanks @DheerendraRathor !

tjstum added a subscriber: tjstum.Oct 7 2016, 8:15 PM

I have a patch to Arc that will use a non-printable control character (INFORMATION SEPARATOR THREE) instead of | for the separator.

Would this be something that you guys would consider taking?

eadler closed this task as a duplicate of T10038: Plan the mid-term pathway for unit-test/linter bindings.Oct 8 2016, 4:50 AM
eadler mentioned this in T10038: Plan the mid-term pathway for unit-test/linter bindings.
Phabricator · Built by Phacility