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

add disabled-by-default warning for self.x = x in __init__ #7

Merged
merged 2 commits into from
Mar 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 32 additions & 7 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,6 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
if i == 0:
# normally, should just be "..."
if isinstance(statement, ast.Pass):
# should disable this if https://github.com/python/typeshed/pull/1009 isn't
# accepted
self.error(statement, Y009)
continue
elif isinstance(statement, ast.Expr) and isinstance(statement.value, ast.Ellipsis):
Expand All @@ -264,10 +262,8 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
continue
# allow assignments in constructor for now
# (though these should probably be changed)
if node.name == '__init__' and isinstance(statement, ast.Assign) and \
isinstance(statement.targets[0], ast.Attribute) and \
isinstance(statement.targets[0].value, ast.Name) and \
statement.targets[0].value.id == 'self':
if node.name == '__init__':
self.error(statement, Y090)
continue
self.error(statement, Y010)

Expand All @@ -292,12 +288,15 @@ class PyiTreeChecker:

tree = attr.ib(default=None)
filename = attr.ib(default='(none)')
options = attr.ib(default=None)

def run(self):
path = Path(self.filename)
if path.suffix == '.pyi':
visitor = PyiVisitor(filename=path)
yield from visitor.run(self.tree)
for error in visitor.run(self.tree):
if self.should_warn(error.message[:4]):
yield error

@classmethod
def add_options(cls, parser):
Expand All @@ -314,13 +313,36 @@ def add_options(cls, parser):
parse_from_config=True,
help="don't patch flake8 with .pyi-aware file checker",
)
parser.extend_default_ignore(DISABLED_BY_DEFAULT)

@classmethod
def parse_options(cls, optmanager, options, extra_args):
"""This is also brittle, only checked with flake8 3.2.1 and master."""
if not options.no_pyi_aware_file_checker:
checker.FileChecker = PyiAwareFileChecker

# Functionality to ignore some warnings. Adapted from flake8-bugbear.
def should_warn(self, code):
"""Returns `True` if flake8-pyi should emit a particular warning.
flake8 overrides default ignores when the user specifies
`ignore = ` in configuration. This is problematic because it means
specifying anything in `ignore = ` implicitly enables all optional
warnings. This function is a workaround for this behavior.
Users should explicitly enable these warnings.
"""
if code[:3] != 'Y09':
# Normal warnings are safe for emission.
return True

if self.options is None:
return True

for i in range(2, len(code) + 1):
if code[:i] in self.options.select:
return True

return False


Y001 = 'Y001 Name of private TypeVar must start with _'
Y002 = 'Y002 If test must be a simple comparison against sys.platform or sys.version_info'
Expand All @@ -332,3 +354,6 @@ def parse_options(cls, optmanager, options, extra_args):
Y008 = 'Y008 Unrecognized platform "{platform}"'
Y009 = 'Y009 Empty body should contain "...", not "pass"'
Y010 = 'Y010 Function body must contain only "..."'
Y090 = 'Y090 Use explicit attributes instead of assignments in __init__'

DISABLED_BY_DEFAULT = [Y090]
2 changes: 1 addition & 1 deletion tests/emptyfunctions.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class GoodClass:

class BadClass:
def __init__(self, b: GoodClass, x: int) -> None:
b.x = x # Y010
b.x = x

def returning(x: int) -> float:
return x / 2 # Y010
Expand Down
3 changes: 3 additions & 0 deletions tests/emptyinit.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class C:
def __init__(self, x: int) -> None:
self.x = x # Y090
26 changes: 19 additions & 7 deletions tests/test_pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
class PyiTestCase(unittest.TestCase):
maxDiff = None # type: int

def checkFile(self, filename: str, pyi_aware: bool) -> Tuple[int, str, str]:
def checkFile(self, filename: str, pyi_aware: bool,
extra_options: Sequence[str] = ()) -> Tuple[int, str, str]:
file_path = Path(__file__).absolute().parent / filename
cmdline = ['flake8', '-j0', str(file_path)]
cmdline = ['flake8', '-j0', *extra_options, str(file_path)]
if not pyi_aware:
cmdline.insert(-1, '--no-pyi-aware-file-checker')
env = os.environ.copy()
Expand All @@ -28,12 +29,13 @@ def checkFile(self, filename: str, pyi_aware: bool) -> Tuple[int, str, str]:
return proc.returncode, stdout_text, stderr_text

def checkFileOutput(self, filename: str, *, pyi_aware: bool = True,
stdout_lines: Sequence[str] = (), stderr_lines: Sequence[str] = ()) -> None:
returncode, stdout, stderr = self.checkFile(filename, pyi_aware=pyi_aware)
stdout_lines: Sequence[str] = (), stderr_lines: Sequence[str] = (),
extra_options: Sequence[str] = ()) -> None:
returncode, stdout, stderr = self.checkFile(filename, pyi_aware=pyi_aware,
extra_options=extra_options)
expected_returncode = 1 if stdout else 0
self.assertEqual(returncode, expected_returncode, stdout)

for (actual, expected_lines) in [(stdout, stdout_lines), (stderr, stderr_lines)]:
for (actual, expected_lines) in [(stderr, stderr_lines), (stdout, stdout_lines)]:
actual = '\n'.join(
line.split('/')[-1] for line in actual.split('\n') if line
)
Expand All @@ -43,6 +45,8 @@ def checkFileOutput(self, filename: str, *, pyi_aware: bool = True,
)
self.assertMultiLineEqual(expected, actual)

self.assertEqual(returncode, expected_returncode, stdout)

def test_vanilla_flake8_not_clean_forward_refs(self) -> None:
stdout_lines = (
"4:22: F821 undefined name 'CStr'",
Expand Down Expand Up @@ -114,12 +118,20 @@ def test_comparisons(self) -> None:
def test_function_def(self) -> None:
stdout_lines = (
'5:5: Y009 Empty body should contain "...", not "pass"',
'16:9: Y010 Function body must contain only "..."',
'19:5: Y010 Function body must contain only "..."',
'23:5: Y010 Function body must contain only "..."',
)
self.checkFileOutput('emptyfunctions.pyi', stdout_lines=stdout_lines)

def test_empty_init(self) -> None:
# should have no output if it's not explicitly selected
self.checkFileOutput('emptyinit.pyi', stdout_lines=())
stdout_lines = (
'3:9: Y090 Use explicit attributes instead of assignments in __init__',
)
self.checkFileOutput('emptyinit.pyi', stdout_lines=stdout_lines,
extra_options=('--select=Y090',))


if __name__ == '__main__':
unittest.main()