-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Refactor to remove circular import in utils/reporter/checker #2809
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
Merged
PCManticore
merged 11 commits into
pylint-dev:master
from
Pierre-Sassoulas:fix-circular-import-in-utils
Mar 29, 2019
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
caaf111
Refactor - Avoid intra-packages circular dependencies for constants
Pierre-Sassoulas d621251
Refactor - Remove circular import between utils and reporter
Pierre-Sassoulas a017319
Refactor - Merge normalize text in utils.py
Pierre-Sassoulas 8fcf693
Refactor - Move refactor.utils function to the utils package
Pierre-Sassoulas f3dabc1
Refactor - Create a new file for ASTWalker unittest
Pierre-Sassoulas ce0fb00
Fix - Move checker.utils tests into their dedicated unittest
Pierre-Sassoulas ab463d0
Refactor - Move function for checker in Checker
Pierre-Sassoulas 1b9e006
Refactor - Create file for BaseChecker
Pierre-Sassoulas bba5eae
Style - Use a single list comprehension instead of a for loop
PCManticore 6e6e655
Refactor - Remove unused and untested code in utils
Pierre-Sassoulas 400a060
Fix - Remove the redefined build-in instead of using a pragma
Pierre-Sassoulas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| # Copyright (c) 2006-2014 LOGILAB S.A. (Paris, FRANCE) <[email protected]> | ||
| # Copyright (c) 2013-2014 Google, Inc. | ||
| # Copyright (c) 2013 [email protected] <[email protected]> | ||
| # Copyright (c) 2014-2017 Claudiu Popa <[email protected]> | ||
| # Copyright (c) 2014 Brett Cannon <[email protected]> | ||
| # Copyright (c) 2014 Arun Persaud <[email protected]> | ||
| # Copyright (c) 2015 Ionel Cristian Maries <[email protected]> | ||
| # Copyright (c) 2016 Moises Lopez <[email protected]> | ||
| # Copyright (c) 2017-2018 Bryce Guinta <[email protected]> | ||
|
|
||
| # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html | ||
| # For details: https://github.com/PyCQA/pylint/blob/master/COPYING | ||
|
|
||
| from typing import Any | ||
|
|
||
| from pylint.config import OptionsProviderMixIn | ||
| from pylint.exceptions import InvalidMessageError | ||
| from pylint.interfaces import UNDEFINED | ||
| from pylint.message import build_message_definition | ||
|
|
||
|
|
||
| class BaseChecker(OptionsProviderMixIn): | ||
|
|
||
| # checker name (you may reuse an existing one) | ||
| name = None # type: str | ||
| # options level (0 will be displaying in --help, 1 in --long-help) | ||
| level = 1 | ||
| # ordered list of options to control the ckecker behaviour | ||
| options = () # type: Any | ||
| # messages issued by this checker | ||
| msgs = {} # type: Any | ||
| # reports issued by this checker | ||
| reports = () # type: Any | ||
| # mark this checker as enabled or not. | ||
| enabled = True | ||
|
|
||
| def __init__(self, linter=None): | ||
| """checker instances should have the linter as argument | ||
|
|
||
| :param ILinter linter: is an object implementing ILinter.""" | ||
| if self.name is not None: | ||
| self.name = self.name.lower() | ||
| OptionsProviderMixIn.__init__(self) | ||
| self.linter = linter | ||
|
|
||
| def add_message( | ||
| self, | ||
| msgid, | ||
| line=None, | ||
| node=None, | ||
| args=None, | ||
| confidence=UNDEFINED, | ||
| col_offset=None, | ||
| ): | ||
| self.linter.add_message(msgid, line, node, args, confidence, col_offset) | ||
|
|
||
| def check_consistency(self) -> None: | ||
| """Check the consistency of msgid. | ||
|
|
||
| msg ids for a checker should be a string of len 4, where the two first | ||
| characters are the checker id and the two last the msg id in this | ||
| checker. | ||
|
|
||
| :raises InvalidMessageError: If the checker id in the messages are not | ||
| always the same. """ | ||
| checker_id = None | ||
| existing_ids = [] | ||
| for message in self.messages: | ||
| if checker_id is not None and checker_id != message.msgid[1:3]: | ||
| error_msg = "Inconsistent checker part in message id " | ||
| error_msg += "'{}' (expected 'x{checker_id}xx' ".format( | ||
| message.msgid, checker_id=checker_id | ||
| ) | ||
| error_msg += "because we already had {existing_ids}).".format( | ||
| existing_ids=existing_ids | ||
| ) | ||
| raise InvalidMessageError(error_msg) | ||
| checker_id = message.msgid[1:3] | ||
| existing_ids.append(message.msgid) | ||
|
|
||
| @property | ||
| def messages(self) -> list: | ||
| return [ | ||
| build_message_definition(self, msgid, msg_tuple) | ||
| for msgid, msg_tuple in sorted(self.msgs.items()) | ||
| ] | ||
|
|
||
| # dummy methods implementing the IChecker interface | ||
|
|
||
| def open(self): | ||
| """called before visiting project (i.e set of modules)""" | ||
|
|
||
| def close(self): | ||
| """called after visiting project (i.e set of modules)""" | ||
|
|
||
|
|
||
| class BaseTokenChecker(BaseChecker): | ||
| """Base class for checkers that want to have access to the token stream.""" | ||
|
|
||
| def process_tokens(self, tokens): | ||
| """Should be overridden by subclasses.""" | ||
| raise NotImplementedError() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ I didn't notice this last time, is it possible to move
build_message_definitionintomessage_definitionitself so we can squash that module with a single exported function?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried but there definitely is a circular import here :
I wanted to move the message building in
BaseCheckerto fix this, but the refactor is humongous see : #2844There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh gotcha, thanks for the extra explanation.