Skip to content

Commit

Permalink
Fix ./pants lint2 for Black and isort (#8877)
Browse files Browse the repository at this point in the history
#8823 broke `./pants lint2` for formatters like Black and Isort because `python_lint_target.py` was not passing the argument `prior_formatter_result_digest` to the constructors for `IsortTarget` and `BlackTarget`.

However, `prior_formatter_result_digest` is only relevant to `fmt2` and the lint logic should never have to deal with the argument.

So, here, we set the field to `Optional` with a default of `None`. `python_fmt_target.py` will ensure that the value is always set when formatting, whereas `python_lint_target.py` will ignore the value as before. This also updates tests to ensure that both code paths are tested.
  • Loading branch information
Eric-Arellano authored and stuhood committed Jan 3, 2020
1 parent 38115ba commit ca9ed54
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 39 deletions.
2 changes: 1 addition & 1 deletion pants.ini
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ pythonpath: [
]

backend_packages: +[
"pants.backend.python.lint.isort",
"pants.backend.docgen",
"pants.contrib.avro",
"pants.contrib.awslambda.python",
Expand All @@ -71,6 +70,7 @@ backend_packages: +[
backend_packages2: [
"pants.backend.project_info",
"pants.backend.python",
"pants.backend.python.lint.isort",
"pants.backend.native",
"internal_backend.rules_for_testing",
]
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/lint/black/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
@dataclass(frozen=True)
class BlackTarget:
target: TargetAdaptor
prior_formatter_result_digest: Digest
prior_formatter_result_digest: Optional[Digest] = None # unused by `lint`


@dataclass(frozen=True)
Expand Down Expand Up @@ -99,7 +99,7 @@ async def create_black_request(
subprocess_encoding_environment: SubprocessEncodingEnvironment,
) -> ExecuteProcessRequest:
target = wrapped_target.target
sources_digest = wrapped_target.prior_formatter_result_digest
sources_digest = wrapped_target.prior_formatter_result_digest or target.sources.snapshot.directory_digest
merged_input_files = await Get[Digest](
DirectoriesToMerge(
directories=(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,33 +70,32 @@ def run_black(
if config is not None:
self.create_file(relpath="pyproject.toml", contents=config)
input_snapshot = self.request_single_product(Snapshot, InputFilesContent(source_files))
target = BlackTarget(
TargetAdaptor(
sources=EagerFilesetWithSpec('test', {'globs': []}, snapshot=input_snapshot),
address=Address.parse("test:target"),
),
prior_formatter_result_digest=input_snapshot.directory_digest,
target_adaptor = TargetAdaptor(
sources=EagerFilesetWithSpec('test', {'globs': []}, snapshot=input_snapshot),
address=Address.parse("test:target"),
)
lint_target = BlackTarget(target_adaptor)
fmt_target = BlackTarget(target_adaptor, prior_formatter_result_digest=input_snapshot.directory_digest)
black_subsystem = global_subsystem_instance(
Black, options={Black.options_scope: {
"config": "pyproject.toml" if config else None,
"args": passthrough_args or [],
}}
)
python_subsystems = [
PythonNativeCode.global_instance(),
PythonSetup.global_instance(),
SubprocessEnvironment.global_instance(),
]
black_setup = self.request_single_product(
BlackSetup,
Params(
black_subsystem,
PythonNativeCode.global_instance(),
PythonSetup.global_instance(),
SubprocessEnvironment.global_instance(),
)
BlackSetup, Params(black_subsystem, *python_subsystems)
)
fmt_and_lint_params = Params(
target, black_setup, PythonSetup.global_instance(), SubprocessEnvironment.global_instance()
lint_result = self.request_single_product(
LintResult, Params(lint_target, black_setup, *python_subsystems)
)
fmt_result = self.request_single_product(
FmtResult, Params(fmt_target, black_setup, *python_subsystems)
)
lint_result = self.request_single_product(LintResult, fmt_and_lint_params)
fmt_result = self.request_single_product(FmtResult, fmt_and_lint_params)
return lint_result, fmt_result

def get_digest(self, source_files: List[FileContent]) -> Digest:
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/lint/isort/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
@dataclass(frozen=True)
class IsortTarget:
target: TargetAdaptor
prior_formatter_result_digest: Digest
prior_formatter_result_digest: Optional[Digest] = None # unused by `lint`


@dataclass(frozen=True)
Expand Down Expand Up @@ -91,7 +91,7 @@ async def create_isort_request(
subprocess_encoding_environment: SubprocessEncodingEnvironment,
) -> ExecuteProcessRequest:
target = wrapped_target.target
sources_digest = wrapped_target.prior_formatter_result_digest
sources_digest = wrapped_target.prior_formatter_result_digest or target.sources.snapshot.directory_digest
merged_input_files = await Get[Digest](
DirectoriesToMerge(
directories=(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,33 +78,32 @@ def run_isort(
if config is not None:
self.create_file(relpath=".isort.cfg", contents=config)
input_snapshot = self.request_single_product(Snapshot, InputFilesContent(source_files))
target = IsortTarget(
TargetAdaptor(
sources=EagerFilesetWithSpec('test', {'globs': []}, snapshot=input_snapshot),
address=Address.parse("test:target"),
),
prior_formatter_result_digest=input_snapshot.directory_digest,
target_adaptor = TargetAdaptor(
sources=EagerFilesetWithSpec('test', {'globs': []}, snapshot=input_snapshot),
address=Address.parse("test:target"),
)
lint_target = IsortTarget(target_adaptor)
fmt_target = IsortTarget(target_adaptor, prior_formatter_result_digest=input_snapshot.directory_digest)
isort_subsystem = global_subsystem_instance(
Isort, options={Isort.options_scope: {
"config": [".isort.cfg"] if config else None,
"args": passthrough_args or [],
}}
)
python_subsystems = [
PythonNativeCode.global_instance(),
PythonSetup.global_instance(),
SubprocessEnvironment.global_instance(),
]
isort_setup = self.request_single_product(
IsortSetup,
Params(
isort_subsystem,
PythonNativeCode.global_instance(),
PythonSetup.global_instance(),
SubprocessEnvironment.global_instance(),
)
IsortSetup, Params(isort_subsystem, *python_subsystems)
)
fmt_and_lint_params = Params(
target, isort_setup, PythonSetup.global_instance(), SubprocessEnvironment.global_instance()
lint_result = self.request_single_product(
LintResult, Params(lint_target, isort_setup, *python_subsystems)
)
fmt_result = self.request_single_product(
FmtResult, Params(fmt_target, isort_setup, *python_subsystems)
)
lint_result = self.request_single_product(LintResult, fmt_and_lint_params)
fmt_result = self.request_single_product(FmtResult, fmt_and_lint_params)
return lint_result, fmt_result

def get_digest(self, source_files: List[FileContent]) -> Digest:
Expand Down

0 comments on commit ca9ed54

Please sign in to comment.