From 92a5803d3074ab4d282c50bb7664c81e60f67a91 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 20 Feb 2013 14:19:55 -0800 Subject: [PATCH] Add linting for syntax brought in by unresolved merge conflicts. Summary: Add linting capability for detecting files which contain syntax introduced by unresolved merge conflicts. The detection is file-type-agnostic (the only requirement is that the file is a text file). Test Plan: Tested in three ways. The first way is to add all three forms of syntax to a file to indicate a merge conflict. HPHP will pick this up as a syntax error before this linter reaches it. The second way is to add the syntax in a comment. In that case, this linter will show three warnings. For example: $ arc lint ArcanistMergeConflictLinter.php >>> Lint for arcanist/src/lint/linter/ArcanistMergeConflictLinter.php: Warning (MERGECONFLICT1) Unresolved merge conflict This syntax indicates there is still an unresolved merge conflict. 20 21 foreach ($lines as $lineno => $line) { 22 /* >>> 23 >>>>>>> 24 25 ======= Warning (MERGECONFLICT1) Unresolved merge conflict This syntax indicates there is still an unresolved merge conflict. 22 /* 23 >>>>>>> 24 >>> 25 ======= 26 27 <<<<<<< Warning (MERGECONFLICT1) Unresolved merge conflict This syntax indicates there is still an unresolved merge conflict. 24 25 ======= 26 >>> 27 <<<<<<< 28 29 */ The last test was to test on various different file types, including JavaScript, PHP, an animated GIF, a PNG, and a Bash file to make sure the file type detection worked. Each of the aforementioned tests passed. Reviewers: vrana, epriestley Reviewed By: epriestley CC: aran, epriestley, Korvin Maniphest Tasks: T2547 Differential Revision: https://secure.phabricator.com/D4966 --- src/__phutil_library_map__.php | 2 + src/lint/engine/PhutilLintEngine.php | 9 ++++ src/lint/linter/ArcanistLinter.php | 5 ++ .../linter/ArcanistMergeConflictLinter.php | 50 +++++++++++++++++++ 4 files changed, 66 insertions(+) create mode 100644 src/lint/linter/ArcanistMergeConflictLinter.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9ea568eb..b9d4598e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -96,6 +96,7 @@ phutil_register_library_map(array( 'ArcanistMercurialAPI' => 'repository/api/ArcanistMercurialAPI.php', 'ArcanistMercurialParser' => 'repository/parser/ArcanistMercurialParser.php', 'ArcanistMercurialParserTestCase' => 'repository/parser/__tests__/ArcanistMercurialParserTestCase.php', + 'ArcanistMergeConflictLinter' => 'lint/linter/ArcanistMergeConflictLinter.php', 'ArcanistNoEffectException' => 'exception/usage/ArcanistNoEffectException.php', 'ArcanistNoEngineException' => 'exception/usage/ArcanistNoEngineException.php', 'ArcanistNoLintLinter' => 'lint/linter/ArcanistNoLintLinter.php', @@ -232,6 +233,7 @@ phutil_register_library_map(array( 'ArcanistMarkCommittedWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistMercurialAPI' => 'ArcanistRepositoryAPI', 'ArcanistMercurialParserTestCase' => 'ArcanistTestCase', + 'ArcanistMergeConflictLinter' => 'ArcanistLinter', 'ArcanistNoEffectException' => 'ArcanistUsageException', 'ArcanistNoEngineException' => 'ArcanistUsageException', 'ArcanistNoLintLinter' => 'ArcanistLinter', diff --git a/src/lint/engine/PhutilLintEngine.php b/src/lint/engine/PhutilLintEngine.php index 267dd5df..5ddb4e6a 100644 --- a/src/lint/engine/PhutilLintEngine.php +++ b/src/lint/engine/PhutilLintEngine.php @@ -57,6 +57,15 @@ class PhutilLintEngine extends ArcanistLintEngine { ->setXHPASTLinter($xhpast_linter) ->setPaths($php_paths); + $merge_conflict_linter = id(new ArcanistMergeConflictLinter()); + + foreach ($paths as $path) { + $merge_conflict_linter->addPath($path); + $merge_conflict_linter->addData($path, $this->loadData($path)); + } + + $linters[] = $merge_conflict_linter; + return $linters; } diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php index 1d5867ab..ec7a7596 100644 --- a/src/lint/linter/ArcanistLinter.php +++ b/src/lint/linter/ArcanistLinter.php @@ -243,4 +243,9 @@ abstract class ArcanistLinter { return self::GRANULARITY_FILE; } + public function isBinaryFile($path) { + // Note that we need the lint engine set before this can be used. + return ArcanistDiffUtils::isHeuristicBinaryFile($this->getData($path)); + } + } diff --git a/src/lint/linter/ArcanistMergeConflictLinter.php b/src/lint/linter/ArcanistMergeConflictLinter.php new file mode 100644 index 00000000..9cbbe70e --- /dev/null +++ b/src/lint/linter/ArcanistMergeConflictLinter.php @@ -0,0 +1,50 @@ +isBinaryFile($path)) { + return; + } + + $lines = phutil_split_lines($this->getData($path), false); + + foreach ($lines as $lineno => $line) { + // An unresolved merge conflict will contain a series of seven + // '<', '=', or '>'. + if (preg_match('/^(>{7}|<{7}|={7})$/', $line)) { + $this->raiseLintAtLine( + $lineno + 1, + 0, + self::LINT_MERGECONFLICT, + "This syntax indicates there is an unresolved merge conflict."); + } + } + } + + public function getLinterName() { + return "MERGECONFLICT"; + } + + public function getLintSeverityMap() { + return array( + self::LINT_MERGECONFLICT => ArcanistLintSeverity::SEVERITY_ERROR + ); + } + + public function getLintNameMap() { + return array( + self::LINT_MERGECONFLICT => "Unresolved merge conflict" + ); + } +}