Skip to content

Commit

Permalink
add disabled-by-default warning for self.x = x in __init__ (#7)
Browse files Browse the repository at this point in the history
* add disabled-by-default warning for self.x = x in __init__

Also:
- Commit a few things that should have been in the last PR but that I forgot to commit
- Test that disabling-by-default works.

* add missing file
  • Loading branch information
JelleZijlstra authored and ambv committed Mar 20, 2017
1 parent 88c34fe commit 9aab9d3
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 15 deletions.
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()

0 comments on commit 9aab9d3

Please sign in to comment.