Skip to content

Commit

Permalink
ARROW-7926: [Dev] Improve "archery lint" UI
Browse files Browse the repository at this point in the history
Rework options structure and semantics.
Individual checks are now boolean options.
A "--all" option is added that enables all checks.
One can easily disable or enable some checks selectively, e.g:
  $ archery lint --rust --r
or:
  $ archery lint --all --no-numpydoc

Closes #6491 from pitrou/ARROW-7926-lint-cli and squashes the following commits:

8f77631 <Antoine Pitrou> ARROW-7926:  Improve "archery lint" UI

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
  • Loading branch information
pitrou authored and kszucs committed Feb 27, 2020
1 parent 3d81e4e commit 34340c6
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 56 deletions.
79 changes: 44 additions & 35 deletions dev/archery/archery/cli.py
Expand Up @@ -16,6 +16,7 @@
# specific language governing permissions and limitations
# under the License.

from collections import namedtuple
import click
import errno
import json
Expand Down Expand Up @@ -203,48 +204,56 @@ def build(ctx, src, build_dir, force, targets, **kwargs):
build.run(target)


@archery.command(short_help="Lint Arrow source directory")
LintCheck = namedtuple('LintCheck', ('option_name', 'help'))

lint_checks = [
LintCheck('clang-format', "Format C++ files with clang-format."),
LintCheck('clang-tidy', "Lint C++ files with clang-tidy."),
LintCheck('cpplint', "Lint C++ files with cpplint."),
LintCheck('iwyu', "Lint C++ files with Include-What-You-Use."),
LintCheck('flake8', "Lint Python files with flake8."),
LintCheck('numpydoc', "Lint Python files with numpydoc."),
LintCheck('cmake-format', "Format CMake files with cmake-format.py."),
LintCheck('rat',
"Check all sources files for license texts via Apache RAT."),
LintCheck('r', "Lint R files."),
LintCheck('rust', "Lint Rust files."),
LintCheck('docker', "Lint Dockerfiles with hadolint."),
]


def decorate_lint_command(cmd):
"""
Decorate the lint() command function to add individual per-check options.
"""
for check in lint_checks:
option = click.option("--{0}/--no-{0}".format(check.option_name),
default=None, help=check.help)
cmd = option(cmd)
return cmd


@archery.command(short_help="Check Arrow source tree for errors")
@click.option("--src", metavar="<arrow_src>", default=ArrowSources.find(),
callback=validate_arrow_sources,
help="Specify Arrow source directory")
@click.option("--with-clang-format", default=True, type=BOOL,
show_default=True,
help="Ensure formatting of C++ files.")
@click.option("--with-cpplint", default=True, type=BOOL,
show_default=True,
help="Ensure linting of C++ files with cpplint.")
@click.option("--with-clang-tidy", default=False, type=BOOL,
show_default=True,
help="Lint C++ with clang-tidy.")
@click.option("--with-iwyu", default=False, type=BOOL,
show_default=True,
help="Lint C++ with Include-What-You-Use (iwyu).")
@click.option("--with-flake8", default=True, type=BOOL,
show_default=True,
help="Lint python files with flake8.")
@click.option("--with-numpydoc", default=False, type=BOOL,
show_default=True, help="Lint python files with numpydoc.")
@click.option("--with-cmake-format", default=True, type=BOOL,
show_default=True,
help="Lint CMakeFiles.txt files with cmake-format.py.")
@click.option("--with-rat", default=True, type=BOOL,
show_default=True,
help="Lint files for license violation via apache-rat.")
@click.option("--with-r", default=True, type=BOOL,
show_default=True,
help="Lint r files.")
@click.option("--with-rust", default=True, type=BOOL,
show_default=True,
help="Lint rust files.")
@click.option("--with-docker", default=True, type=BOOL,
show_default=True,
help="Lint docker images with hadolint.")
@click.option("--fix", is_flag=True, type=BOOL, default=False,
help="Toggle fixing the lint errors if the linter supports it.")
@click.option("-a", "--all", is_flag=True, default=False,
help="Enable all checks.")
@decorate_lint_command
@click.pass_context
def lint(ctx, src, **kwargs):
def lint(ctx, src, fix, **checks):
if checks.pop('all'):
# "--all" is given => enable all non-selected checks
for k, v in checks.items():
if v is None:
checks[k] = True
if not any(checks.values()):
raise click.UsageError(
"Need to enable at least one lint check (try --help)")
try:
linter(src, **kwargs)
linter(src, fix, **checks)
except LintValidationException:
sys.exit(1)

Expand Down
38 changes: 18 additions & 20 deletions dev/archery/archery/utils/lint.py
Expand Up @@ -143,8 +143,9 @@ def python_numpydoc(symbols=None, whitelist=None, blacklist=None):
}
try:
numpydoc = NumpyDoc(symbols)
except RuntimeError:
logger.error('Numpydoc is not available')
except RuntimeError as e:
logger.error(str(e))
yield LintResult(success=False)
return

results = numpydoc.validate(
Expand Down Expand Up @@ -282,12 +283,9 @@ def docker_linter(src):
cwd=src.path))


def linter(src, with_clang_format=True, with_cpplint=True,
with_clang_tidy=False, with_iwyu=False,
with_flake8=True, with_numpydoc=False, with_cmake_format=True,
with_rat=True, with_r=True, with_rust=True,
with_docker=True,
fix=False):
def linter(src, fix=False, *, clang_format=False, cpplint=False,
clang_tidy=False, iwyu=False, flake8=False, numpydoc=False,
cmake_format=False, rat=False, r=False, rust=False, docker=False):
"""Run all linters."""
with tmpdir(prefix="arrow-lint-") as root:
build_dir = os.path.join(root, "cpp-build")
Expand All @@ -297,33 +295,33 @@ def linter(src, with_clang_format=True, with_cpplint=True,
# errors to the user.
results = []

if with_clang_format or with_cpplint or with_clang_tidy or with_iwyu:
if clang_format or cpplint or clang_tidy or iwyu:
results.extend(cpp_linter(src, build_dir,
clang_format=with_clang_format,
cpplint=with_cpplint,
clang_tidy=with_clang_tidy,
iwyu=with_iwyu,
clang_format=clang_format,
cpplint=cpplint,
clang_tidy=clang_tidy,
iwyu=iwyu,
fix=fix))

if with_flake8:
if flake8:
results.extend(python_linter(src))

if with_numpydoc:
if numpydoc:
results.extend(python_numpydoc())

if with_cmake_format:
if cmake_format:
results.extend(cmake_linter(src, fix=fix))

if with_rat:
if rat:
results.extend(rat_linter(src, root))

if with_r:
if r:
results.extend(r_linter(src))

if with_rust:
if rust:
results.extend(rust_linter(src))

if with_docker:
if docker:
results.extend(docker_linter(src))

# Raise error if one linter failed, ensuring calling code can exit with
Expand Down
4 changes: 3 additions & 1 deletion docker-compose.yml
Expand Up @@ -1049,7 +1049,9 @@ services:
<<: *ccache
volumes: *ubuntu-volumes
command: >
/bin/bash -c "pip install -e /arrow/dev/archery && archery lint"
/bin/bash -c "
pip install -e /arrow/dev/archery &&
archery lint --all --no-clang-tidy --no-numpydoc"
######################### Integration Tests #################################

Expand Down

0 comments on commit 34340c6

Please sign in to comment.