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.

Sign up for GitHub

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

Jump to bottom

Allow using --import option with Autoflake #16192

Merged
merged 9 commits into from
Jul 22, 2022
Merged

Allow using --import option with Autoflake #16192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f9f10b7
Remove hardcoded argument from autoflake linter
Jul 15, 2022
d7ac781
pass default through super call
Jul 15, 2022
69ee9af
add comment about why default was added
Jul 15, 2022
b472249
add issue to comment
Jul 15, 2022
3d10242
Update src/python/pants/option/option_types.py
Eric-Arellano Jul 19, 2022
d530ae7
Another attempt to fix the type annotation
Jul 21, 2022
e64047a
add type ignore and use None instead of []
Jul 21, 2022
8f65014
Merge branch 'main' of github.com:pantsbuild/pants into tdimiduk/main
Eric-Arellano Jul 22, 2022
6828a98
Fix fmt
Eric-Arellano Jul 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/python/pants/backend/python/lint/autoflake/rules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ async def autoflake_fmt(request: AutoflakeRequest, autoflake: Autoflake) -> FmtR
autoflake_pex,
argv=(
"--in-place",
"--remove-all-unused-imports",
*autoflake.args,
*request.snapshot.files,
),
Expand Down
8 changes: 7 additions & 1 deletion src/python/pants/backend/python/lint/autoflake/subsystem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@ class Autoflake(PythonToolBase):
default_lockfile_url = git_url(default_lockfile_path)

skip = SkipOption("fmt", "lint")
args = ArgsListOption(example="--target-version=py37 --quiet")
args = ArgsListOption(
example="--remove-all-unused-imports --target-version=py37 --quiet",
# This argument was previously hardcoded. Moved it a default argument
# to allow it to be overridden while maintaining the existing api.
# See: https://github.com/pantsbuild/pants/issues/16193
default=["--remove-all-unused-imports"],
)
export = ExportToolOption()


Expand Down
4 changes: 3 additions & 1 deletion src/python/pants/option/option_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def __new__(
cls,
flag_name: str | None = None,
*,
default: _MaybeDynamicT[list[_ListMemberT]] = [],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discovered there is already a check on line 234 to put in [] if default is falsy. Seems like making the default here Optional and passing in None is preferable to using the mutable default argument. Happy to back it out if we don't want to change the signature here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, since we can't get the type check to work anyway, better to not do the bad thing with the mutable default for no purpose.

default: _MaybeDynamicT[list[_ListMemberT]] | None = [],
help: _HelpT,
# Additional bells/whistles
register_if: _RegisterIfFuncT | None = None,
Expand Down Expand Up @@ -789,6 +789,7 @@ def __new__(
# This should be set when callers can alternatively use "--" followed by the arguments,
# instead of having to provide "--[scope]-args='--arg1 --arg2'".
passthrough: bool | None = None,
default: _MaybeDynamicT[list[_ListMemberT]] | None = None,
):
if extra_help:
extra_help = "\n\n" + extra_help
Expand All @@ -802,6 +803,7 @@ def __new__(
"""
)
),
default=default, # type: ignore[arg-type]
)
if passthrough is not None:
instance._extra_kwargs["passthrough"] = passthrough
Expand Down