extract new violation - WPS450 from WPS436 #1118
Conversation
Pull Request Test Coverage Report for Build 2511
馃挍 - Coveralls |
| DottedRawImportViolation | ||
| NestedImportViolation | ||
| ProtectedModuleViolation, | ||
| SameAliasImportViolation | ||
| VagueImportViolation, |
| DottedRawImportViolation | ||
| NestedImportViolation | ||
| ProtectedModuleViolation, |
| @@ -27,7 +28,7 @@ | |||
| ErrorCallback = Callable[[BaseViolation], None] # TODO: alias and move | |||
|
|
|||
|
|
|||
| @final | |||
| @final # noqa: WPS214 | |||
There was a problem hiding this comment.
Can we do something with this violation here?
Can we refactor this class to be more simple? Or just split it?
There was a problem hiding this comment.
In the latest commit I splitted this validator into 3 classes:
ast.Importvalidatorast.ImportFromvalidator- base class for both above mentioned validators
If you have any ideas how to make it clearer please let me know.
| """Validator of ``ast.Import`` nodes.""" | ||
|
|
||
| def validate(self, node: AnyImport) -> None: | ||
| node = cast(ast.Import, node) |
There was a problem hiding this comment.
I dislike this casting :D
Do you know if there is any better way to type such strict subclasses which provides implementation only for one type of the type from Union defined in base class?
| """Validator of ``ast.Import`` nodes.""" | ||
|
|
||
| def validate(self, node: AnyImport) -> None: | ||
| node = cast(ast.Import, node) |
There was a problem hiding this comment.
We try not use cast. It should be a bug in Python typing to actually use it.
Can you please refactor this place once again?
There was a problem hiding this comment.
Sure, but to do that I need to use AnyImport everywhere instead of concrete import's type that is being validated in given method, is that ok?
e,g def _check_dotted_raw_import(self, node: AnyImport) -> None: instead of def _check_dotted_raw_import(self, node: ast.Import) -> None:
| @@ -39,30 +40,18 @@ def __init__( | |||
| self._error_callback = error_callback | |||
| self._options = options | |||
|
|
|||
| def check_nested_import(self, node: AnyImport) -> None: | |||
| @abc.abstractmethod | |||
| def validate(self, node: AnyImport): | |||
There was a problem hiding this comment.
We can just name this method differently. Like _validate_any_import and then call it in specific .validate method instead of super().validate()
There was a problem hiding this comment.
Sure, but then we need to resign from declaring .validate() as abstractmethod and just provide this method in 2 concrete validators to satisfy mypy, is that what you suggest?
I have made things!
Checklist
CHANGELOG.mdRelated issues
Closes: #1113
馃檹 Please, if you or your company is finding wemake-python-styleguide valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/wemake-python-styleguide. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.