Skip to content

Commit

Permalink
[LINTER] Add a YAML linter
Browse files Browse the repository at this point in the history
Summary:
This uses a relaxed rule set for now, which does not enforce a strict
number of indentation spaces but instead check the file is consistently
indented. The maximum line lenght rule is also disabled to accomodate
the scripts in the gitian descriptors.

Test Plan:
Should return no error:
  arc lint --everything
Revert the changes to the .travis.yml from this diff then run:
  arc lint
and check the indentation errors are catched by the linter.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D7358
  • Loading branch information
Fabcien committed Sep 4, 2020
1 parent 491ad85 commit b278e1a
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 14 deletions.
4 changes: 4 additions & 0 deletions .arclint
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@
"exclude": [
"(^src/(crypto/ctaes|secp256k1|univalue|leveldb)/)"
]
},
"yamllint": {
"type": "yamllint",
"include": "(\\.(yml|yaml)$)"
}
}
}
7 changes: 7 additions & 0 deletions .yamllint
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
extends: relaxed

rules:
# Some YML files embed a script portion, for which the line length rule would
# decrease the readability.
line-length: disable
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ will have to install the following:

On Ubuntu (>= 18.04+updates):
```
sudo apt-get install clang-format-8 clang-tidy-8 clang-tools-8 cppcheck python-autopep8 flake8 php-codesniffer
sudo apt-get install clang-format-8 clang-tidy-8 clang-tools-8 cppcheck python-autopep8 flake8 php-codesniffer yamllint
```

On Debian (>= 10), the clang-8 family of tools is available from the `buster-backports` repository:
Expand Down
2 changes: 1 addition & 1 deletion arcanist/.phutil_module_cache

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions arcanist/__phutil_library_map__.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
'StdintLinter' => 'linter/StdintLinter.php',
'TestsLinter' => 'linter/TestsLinter.php',
'WhitespaceLinter' => 'linter/WhitespaceLinter.php',
'YamllintLinter' => 'linter/YamllintLinter.php',
),
'function' => array(),
'xmap' => array(
Expand Down Expand Up @@ -78,5 +79,6 @@
'StdintLinter' => 'ArcanistLinter',
'TestsLinter' => 'ArcanistExternalLinter',
'WhitespaceLinter' => 'ArcanistLinter',
'YamllintLinter' => 'ArcanistExternalLinter',
),
));
82 changes: 82 additions & 0 deletions arcanist/linter/YamllintLinter.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php

/**
* Uses the yamllint tool to lint the YAML files.
*/
final class YamllintLinter extends ArcanistExternalLinter {

const YAMLLINT_SEVERITY_MAP = array(
'warning' => ArcanistLintSeverity::SEVERITY_WARNING,
'error' => ArcanistLintSeverity::SEVERITY_ERROR,
);

public function getInfoName() {
return 'yamllint';
}

public function getInfoDescription() {
return pht('Use yamllint for linting YAML files.');
}

public function getLinterName() {
return 'YAMLLINT';
}

public function getLinterConfigurationName() {
return 'yamllint';
}

public function getDefaultBinary() {
return 'yamllint';
}

public function getInstallInstructions() {
return pht('Please install yamllint and make sure it is in your $PATH');
}

public function shouldExpectCommandErrors() {
return true;
}

public function getVersion() {
list($stdout) = execx('%C --version', $this->getExecutableCommand());

$regex = '/^yamllint (?P<version>\d+\.\d+\.\d+)/m';
if (preg_match($regex, $stdout, $matches)) {
return $matches['version'];
}

return false;
}

private function getSeverity($severity) {
if (array_key_exists($severity, self::YAMLLINT_SEVERITY_MAP)) {
return self::YAMLLINT_SEVERITY_MAP[$severity];
}

return ArcanistLintSeverity::SEVERITY_ERROR;
}

protected function parseLinterOutput($path, $err, $stdout, $stderr) {
$messages = array();

$pattern = '/(\d+):(\d+)\s+(\w+)\s+(.+) \((.+)\)/';
if (preg_match_all($pattern, $stdout, $errors, PREG_SET_ORDER)) {
foreach ($errors as $error) {
list(, $line, $char, $severity, $message, $category) = $error;

$messages[] = id(new ArcanistLintMessage())
->setGranularity(ArcanistLinter::GRANULARITY_FILE)
->setPath($path)
->setLine(intval($line))
->setChar(intval($char))
->setCode($category)
->setSeverity($this->getSeverity($severity))
->setName('yamllint found an issue:')
->setDescription($message);
}
}

return $messages;
}
}
1 change: 1 addition & 0 deletions contrib/teamcity/setup-debian-buster.sh
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ PACKAGES=(
software-properties-common
tar
wget
yamllint
wine
)

Expand Down
24 changes: 12 additions & 12 deletions src/secp256k1/.travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,15 @@ jobs:
- CMAKE_EXTRA_FLAGS=-DCMAKE_C_FLAGS=-DVALGRIND CMAKE_TARGET="secp256k1-tests secp256k1-exhaustive_tests"

script:
- ./travis/build_autotools.sh
- ./travis/build_cmake.sh
- # travis_wait extends the 10 minutes without output allowed (https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received)
- # the `--error-exitcode` is required to make the test fail if valgrind found errors, otherwise it'll return 0 (http://valgrind.org/docs/manual/manual-core.html)
- if [ -n "$VALGRIND" ]; then
travis_wait 30 valgrind --error-exitcode=42 ./buildautotools/tests 16 &&
travis_wait 30 valgrind --error-exitcode=42 ./buildautotools/exhaustive_tests;
fi
- if [ -n "$VALGRIND" ]; then
travis_wait 30 valgrind --error-exitcode=42 ./buildcmake/secp256k1-tests 16 &&
travis_wait 30 valgrind --error-exitcode=42 ./buildcmake/secp256k1-exhaustive_tests;
fi
- ./travis/build_autotools.sh
- ./travis/build_cmake.sh
- # travis_wait extends the 10 minutes without output allowed (https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received)
- # the `--error-exitcode` is required to make the test fail if valgrind found errors, otherwise it'll return 0 (http://valgrind.org/docs/manual/manual-core.html)
- if [ -n "$VALGRIND" ]; then
travis_wait 30 valgrind --error-exitcode=42 ./buildautotools/tests 16 &&
travis_wait 30 valgrind --error-exitcode=42 ./buildautotools/exhaustive_tests;
fi
- if [ -n "$VALGRIND" ]; then
travis_wait 30 valgrind --error-exitcode=42 ./buildcmake/secp256k1-tests 16 &&
travis_wait 30 valgrind --error-exitcode=42 ./buildcmake/secp256k1-exhaustive_tests;
fi

0 comments on commit b278e1a

Please sign in to comment.