From 05c12eb9d9cb615112e754407b32a8469e98e100 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 11 Jan 2016 17:18:58 -0800 Subject: [PATCH 1/3] If the Script-and-Regex linter captures no "line" text, treat the message as affecting the entire file Summary: Fixes T10124. Test Plan: Added this "linter" to `.arclint`: ``` "thing": { "type": "script-and-regex", "script-and-regex.script": "echo every file is very bad #", "script-and-regex.regex": "/^(?P.*)/" } ``` ...to produce this diff. Also made a variant of it which matches lines to make sure that still works. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10124 Differential Revision: https://secure.phabricator.com/D15000 --- src/lint/linter/ArcanistScriptAndRegexLinter.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/lint/linter/ArcanistScriptAndRegexLinter.php b/src/lint/linter/ArcanistScriptAndRegexLinter.php index ca8883cb..0c3d9d9a 100644 --- a/src/lint/linter/ArcanistScriptAndRegexLinter.php +++ b/src/lint/linter/ArcanistScriptAndRegexLinter.php @@ -108,7 +108,8 @@ * not specified, defaults to the linted file. It is generally not necessary * to capture this unless the linter can raise messages in files other than * the one it is linting. - * - `line` (optional) The line number of the message. + * - `line` (optional) The line number of the message. If no text is + * captured, the message is assumed to affect the entire file. * - `char` (optional) The character offset of the message. * - `offset` (optional) The byte offset of the message. If captured, this * supersedes `line` and `char`. @@ -324,7 +325,7 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter { * Get the line and character of the message from the regex match. * * @param dict Captured groups from regex. - * @return pair Line and character of the message. + * @return pair Line and character of the message. * * @task parse */ @@ -337,10 +338,13 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter { } $line = idx($match, 'line'); - if ($line) { + if (strlen($line)) { $line = (int)$line; + if (!$line) { + $line = 1; + } } else { - $line = 1; + $line = null; } $char = idx($match, 'char'); From 22af67c1c05e47546b57e8d20976b0688724fb9f Mon Sep 17 00:00:00 2001 From: Michael Akinde Date: Thu, 14 Jan 2016 06:59:10 -0800 Subject: [PATCH 2/3] Minor fixes to arcanist cpplint Summary: This fixes the regex in "getLintCode..." so that it accepts lint codes such as `build/c++11` which have become valid lint codes in later versions of cpplint. It also corrects the install instructions for the linter (Google's style guide is no longer available on SVN and has been migrated to Github). Test Plan: For the Regex: - Create an .arclint in a project such as: ``` { "linters": { "cpplint": { "type": "cpplint", "severity": { "build/c++11": "advice" } } } } ``` - Run `arc lint` with the existing linter. This should fail. Patch the linter, and this should now be accepted. For the Instructions - Verify the download location `wget https://raw.github.com/google/styleguide/gh-pages/cpplint/cpplint.py` Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Maniphest Tasks: T10118 Differential Revision: https://secure.phabricator.com/D15019 --- src/lint/linter/ArcanistCpplintLinter.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/lint/linter/ArcanistCpplintLinter.php b/src/lint/linter/ArcanistCpplintLinter.php index 0b3416d9..70a387e0 100644 --- a/src/lint/linter/ArcanistCpplintLinter.php +++ b/src/lint/linter/ArcanistCpplintLinter.php @@ -19,9 +19,10 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter { public function getInstallInstructions() { return pht( - 'Install cpplint.py using `%s`.', - 'wget http://google-styleguide.googlecode.com'. - '/svn/trunk/cpplint/cpplint.py'); + 'Install cpplint.py using `%s`, and place it in your path with the'. + 'appropriate permissions set.', + 'wget https://raw.github.com'. + '/google/styleguide/gh-pages/cpplint/cpplint.py'); } protected function getDefaultMessageSeverity($code) { @@ -60,7 +61,7 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter { } protected function getLintCodeFromLinterConfigurationKey($code) { - if (!preg_match('@^[a-z_]+/[a-z_]+$@', $code)) { + if (!preg_match('@^[a-z_]+/[a-z0-9_+]+$@', $code)) { throw new Exception( pht( 'Unrecognized lint message code "%s". Expected a valid cpplint '. From b87138356a9c71ad5e08eaa05399d0233529bc71 Mon Sep 17 00:00:00 2001 From: Michael Akinde Date: Fri, 15 Jan 2016 08:19:23 -0800 Subject: [PATCH 3/3] Cleans up some minor issues in cpplint Summary: - Corrected typo in install instructions - Sets the default binary to cpplint.py Test Plan: - Running the unit tests. You may need to check your test environment is set up with the latest cpplint.py available, since the default binary is being changed. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Maniphest Tasks: T10157 Differential Revision: https://secure.phabricator.com/D15030 --- src/lint/linter/ArcanistCpplintLinter.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lint/linter/ArcanistCpplintLinter.php b/src/lint/linter/ArcanistCpplintLinter.php index 70a387e0..f9c6ddf5 100644 --- a/src/lint/linter/ArcanistCpplintLinter.php +++ b/src/lint/linter/ArcanistCpplintLinter.php @@ -14,12 +14,12 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter { } public function getDefaultBinary() { - return 'cpplint'; + return 'cpplint.py'; } public function getInstallInstructions() { return pht( - 'Install cpplint.py using `%s`, and place it in your path with the'. + 'Install cpplint.py using `%s`, and place it in your path with the '. 'appropriate permissions set.', 'wget https://raw.github.com'. '/google/styleguide/gh-pages/cpplint/cpplint.py');