Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flake8 output file support. #10371

Merged
merged 10 commits into from Jul 21, 2020
36 changes: 30 additions & 6 deletions src/python/pants/backend/python/lint/flake8/rules.py
Expand Up @@ -15,14 +15,14 @@
from pants.backend.python.subsystems import python_native_code, subprocess_environment
from pants.backend.python.subsystems.subprocess_environment import SubprocessEncodingEnvironment
from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources
from pants.core.goals.lint import LintRequest, LintResult, LintResults
from pants.core.goals.lint import LintOptions, LintRequest, LintResult, LintResultFile, LintResults
from pants.core.util_rules import determine_source_files, strip_source_roots
from pants.core.util_rules.determine_source_files import (
AllSourceFilesRequest,
SourceFiles,
SpecifiedSourceFilesRequest,
)
from pants.engine.fs import Digest, MergeDigests, PathGlobs, Snapshot
from pants.engine.fs import Digest, MergeDigests, PathGlobs, Snapshot, SnapshotSubset
from pants.engine.process import FallibleProcessResult, Process
from pants.engine.rules import SubsystemRule, rule
from pants.engine.selectors import Get, MultiGet
Expand Down Expand Up @@ -51,10 +51,14 @@ class Flake8Partition:
interpreter_constraints: PexInterpreterConstraints


def generate_args(*, specified_source_files: SourceFiles, flake8: Flake8) -> Tuple[str, ...]:
def generate_args(
*, specified_source_files: SourceFiles, flake8: Flake8, output_file: Optional[str]
) -> Tuple[str, ...]:
args = []
if flake8.options.config is not None:
args.append(f"--config={flake8.options.config}")
if output_file:
args.append(f"--output-file={output_file}")
args.extend(flake8.options.args)
args.extend(specified_source_files.files)
return tuple(args)
Expand All @@ -64,6 +68,7 @@ def generate_args(*, specified_source_files: SourceFiles, flake8: Flake8) -> Tup
async def flake8_lint_partition(
partition: Flake8Partition,
flake8: Flake8,
lint_options: LintOptions,
python_setup: PythonSetup,
subprocess_encoding_environment: SubprocessEncodingEnvironment,
) -> LintResult:
Expand Down Expand Up @@ -117,19 +122,38 @@ async def flake8_lint_partition(
address_references = ", ".join(
sorted(field_set.address.reference() for field_set in partition.field_sets)
)

output_dir = lint_options.output_dir
output_path = output_dir / "flake8_report.txt" if output_dir else None
flake8_args = generate_args(
specified_source_files=specified_source_files,
flake8=flake8,
output_file=output_path.name if output_path else None,
)
process = requirements_pex.create_process(
python_setup=python_setup,
subprocess_encoding_environment=subprocess_encoding_environment,
pex_path="./flake8.pex",
pex_args=generate_args(specified_source_files=specified_source_files, flake8=flake8),
pex_args=flake8_args,
output_files=(output_path.name,) if output_path else None,
input_digest=input_digest,
description=(
f"Run Flake8 on {pluralize(len(partition.field_sets), 'target')}: {address_references}."
),
)
result = await Get(FallibleProcessResult, Process, process)
return LintResult.from_fallible_process_result(result, linter_name="Flake8")
results_file = None

if output_path:
report_file_snapshot = await Get(
Snapshot, SnapshotSubset(result.output_digest, PathGlobs([output_path.name]))
)
if report_file_snapshot.is_empty or len(report_file_snapshot.files) != 1:
raise Exception(f"Unexpected report file snapshot: {report_file_snapshot}")
results_file = LintResultFile(output_path=output_path, digest=report_file_snapshot.digest)

return LintResult.from_fallible_process_result(
result, linter_name="Flake8", results_file=results_file
)


@rule(desc="Lint using Flake8")
Expand Down
Expand Up @@ -7,12 +7,13 @@
from pants.backend.python.lint.flake8.rules import rules as flake8_rules
from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonLibrary
from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress
from pants.core.goals.lint import LintResults
from pants.core.goals.lint import LintOptions, LintResults
from pants.engine.addresses import Address
from pants.engine.fs import FileContent
from pants.engine.fs import DigestContents, FileContent
from pants.engine.rules import RootRule
from pants.engine.selectors import Params
from pants.engine.target import TargetWithOrigin
from pants.testutil.engine.util import create_subsystem
from pants.testutil.external_tool_test_base import ExternalToolTestBase
from pants.testutil.interpreter_selection_utils import skip_unless_python27_and_python3_present
from pants.testutil.option.util import create_options_bootstrapper
Expand All @@ -26,7 +27,7 @@ class Flake8IntegrationTest(ExternalToolTestBase):

@classmethod
def rules(cls):
return (*super().rules(), *flake8_rules(), RootRule(Flake8Request))
return (*super().rules(), *flake8_rules(), RootRule(Flake8Request), RootRule(LintOptions))

def make_target_with_origin(
self,
Expand All @@ -53,6 +54,7 @@ def run_flake8(
passthrough_args: Optional[str] = None,
skip: bool = False,
additional_args: Optional[List[str]] = None,
reports_dir: Optional[str] = None,
) -> LintResults:
args = ["--backend-packages=pants.backend.python.lint.flake8"]
if config:
Expand All @@ -68,6 +70,7 @@ def run_flake8(
LintResults,
Params(
Flake8Request(Flake8FieldSet.create(tgt) for tgt in targets),
create_subsystem(LintOptions, reports_dir=reports_dir), # type: ignore[type-var]
create_options_bootstrapper(args=args),
),
)
Expand All @@ -78,13 +81,15 @@ def test_passing_source(self) -> None:
assert len(result) == 1
assert result[0].exit_code == 0
assert result[0].stdout.strip() == ""
assert result[0].results_file is None

def test_failing_source(self) -> None:
target = self.make_target_with_origin([self.bad_source])
result = self.run_flake8([target])
assert len(result) == 1
assert result[0].exit_code == 1
assert "bad.py:1:1: F401" in result[0].stdout
assert result[0].results_file is None

def test_mixed_sources(self) -> None:
target = self.make_target_with_origin([self.good_source, self.bad_source])
Expand Down Expand Up @@ -173,3 +178,14 @@ def test_3rdparty_plugin(self) -> None:
assert len(result) == 1
assert result[0].exit_code == 1
assert "bad.py:1:1: PB11" in result[0].stdout

def test_output_file(self) -> None:
target = self.make_target_with_origin([self.bad_source])
result = self.run_flake8([target], reports_dir=".")
assert len(result) == 1
assert result[0].exit_code == 1
assert result[0].stdout.strip() == ""
assert result[0].results_file is not None
output_files = self.request_single_product(DigestContents, result[0].results_file.digest)
assert len(output_files) == 1
assert "bad.py:1:1: F401" in output_files[0].content.decode()
44 changes: 42 additions & 2 deletions src/python/pants/core/goals/lint.py
Expand Up @@ -3,7 +3,8 @@

import itertools
from dataclasses import dataclass
from typing import Iterable
from pathlib import Path
from typing import Iterable, Optional

from pants.core.goals.style_request import StyleRequest
from pants.core.util_rules.filter_empty_sources import (
Expand All @@ -12,6 +13,7 @@
)
from pants.engine.collection import Collection
from pants.engine.console import Console
from pants.engine.fs import Digest, DirectoryToMaterialize, Workspace
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.process import FallibleProcessResult
from pants.engine.rules import goal_rule
Expand All @@ -21,16 +23,27 @@
from pants.util.strutil import strip_v2_chroot_path


@dataclass(frozen=True)
class LintResultFile:
output_path: Path
digest: Digest


@dataclass(frozen=True)
class LintResult:
exit_code: int
stdout: str
stderr: str
linter_name: str
results_file: Optional[LintResultFile]

@staticmethod
def from_fallible_process_result(
process_result: FallibleProcessResult, *, linter_name: str, strip_chroot_path: bool = False
process_result: FallibleProcessResult,
*,
linter_name: str,
strip_chroot_path: bool = False,
results_file: Optional[LintResultFile] = None,
) -> "LintResult":
def prep_output(s: bytes) -> str:
return strip_v2_chroot_path(s) if strip_chroot_path else s.decode()
Expand All @@ -40,7 +53,19 @@ def prep_output(s: bytes) -> str:
stdout=prep_output(process_result.stdout),
stderr=prep_output(process_result.stderr),
linter_name=linter_name,
results_file=results_file,
)

def materialize(self, console: Console, workspace: Workspace) -> None:
if not self.results_file:
return
output_path = self.results_file.output_path
workspace.materialize_directory(
DirectoryToMaterialize(
self.results_file.digest, path_prefix=output_path.parent.as_posix(),
)
)
console.print_stdout(f"Wrote {self.linter_name} report to: {output_path.as_posix()}")


class LintResults(Collection[LintResult]):
Expand Down Expand Up @@ -85,6 +110,19 @@ def register_options(cls, register) -> None:
"faster than `--no-per-target-caching` for your use case."
),
)
register(
"--reports-dir",
type=str,
metavar="<DIR>",
default=None,
advanced=True,
help="Specifying a directory causes linter that support it to write report files into this directory",
)

@property
def output_dir(self) -> Optional[Path]:
reports_dir = self.values.reports_dir
return Path(reports_dir) if reports_dir else None


class Lint(Goal):
Expand All @@ -94,6 +132,7 @@ class Lint(Goal):
@goal_rule
async def lint(
console: Console,
workspace: Workspace,
targets_with_origins: TargetsWithOrigins,
options: LintOptions,
union_membership: UnionMembership,
Expand Down Expand Up @@ -145,6 +184,7 @@ async def lint(
console.print_stderr(result.stderr)
if result != sorted_results[-1]:
console.print_stderr("")
result.materialize(console, workspace)
if result.exit_code != 0:
exit_code = result.exit_code

Expand Down
6 changes: 5 additions & 1 deletion src/python/pants/core/goals/lint_test.py
Expand Up @@ -12,6 +12,7 @@
FieldSetsWithSourcesRequest,
)
from pants.engine.addresses import Address
from pants.engine.fs import Workspace
from pants.engine.target import (
FieldSetWithOrigin,
Sources,
Expand Down Expand Up @@ -57,6 +58,7 @@ def lint_results(self) -> LintResults:
self.stdout(addresses),
"",
linter_name=self.linter_name,
results_file=None,
)
]
)
Expand Down Expand Up @@ -131,20 +133,22 @@ def make_target_with_origin(address: Optional[Address] = None) -> TargetWithOrig
origin=SingleAddress(directory=address.spec_path, name=address.target_name),
)

@staticmethod
def run_lint_rule(
self,
*,
lint_request_types: List[Type[LintRequest]],
targets: List[TargetWithOrigin],
per_target_caching: bool,
include_sources: bool = True,
) -> Tuple[int, str]:
console = MockConsole(use_colors=False)
workspace = Workspace(self.scheduler)
union_membership = UnionMembership({LintRequest: lint_request_types})
result: Lint = run_rule(
lint,
rule_args=[
console,
workspace,
TargetsWithOrigins(targets),
create_goal_subsystem(LintOptions, per_target_caching=per_target_caching),
union_membership,
Expand Down