Page MenuHomePhabricator

ArcanistLinter's addLintMessage incorrectly normalizes path on Windows
Closed, ResolvedPublic

Description

Description:

On Windows, the code that sits inside addLintMessage incorrectly normalizes the path to contain \ path separators:

final protected function addLintMessage(ArcanistLintMessage $message) {
  $root = $this->getProjectRoot();
  $path = Filesystem::resolvePath($message->getPath(), $root);
  $message->setPath(Filesystem::readablePath($path, $root));

  $this->messages[] = $message;
  return $message;
}

When it normalizes the path like this, the path property no longer matches for "are lint warnings relevant for change" logic because that is still using / separators in the path.

Replication Steps:

  1. Obtain Windows
  2. Add extra debugging logic to ArcanistLintEngine's isRelevantMessage function to show the list of paths in $this->changedLines and the path of the message.
  3. Run arc diff or arc lint on a repository where you've made changes that would raise lint warnings

Expected Results:

The lint warning should be raised.

Actual Results:

You can observe the path separators do not match, and that no lint warnings are raised.

Fix:

I believe the fix should be to change this function to:

final protected function addLintMessage(ArcanistLintMessage $message) {
  $root = $this->getProjectRoot();
  $path = Filesystem::resolvePath($message->getPath(), $root);
  $readable_path = Filesystem::readablePath($path, $root);
  $readable_path = str_replace("\\", "/", $readable_path);
  $message->setPath($readable_path);

  $this->messages[] = $message;
  return $message;
}