search

nejucomo / onslaught

Run style and static checks against a python codebase, execute automated tests with coverage.
0 stars 1 forks source link

add "check-miscaptures" and "check-interfaces" from Tahoe-LAFS #1

Open daira opened 9 years ago

daira commented 9 years ago

check-miscaptures (https://github.com/tahoe-lafs/tahoe-lafs/blob/master/misc/coding_tools/check-miscaptures.py) checks for function definitions and lambdas within a loop (including list comprehensions) that capture the loop variable in a way that is probably incorrect. Usually, the intention is to capture the value of the loop variable on the iteration in which the function/lambda abstraction was created, but Python will actually use the value when it is called.

check-interfaces (https://github.com/tahoe-lafs/tahoe-lafs/blob/master/misc/coding_tools/check-interfaces.py) is specific to code using zope.interface. It checks for inconsistencies between an interface definition and any classes that declare that they implement the interface. Unlike zope.interface.verify, it reports all errors nicely rather than bailing on the first one.

nejucomo commented 9 years ago

I consider check-interfaces.py too specialized for this tool. I'd like for the tests it executes to be general to all python (2.* for now). One possibility for additional or specialized tests, is to simply run them before onslaught against the source directory. If tests need to be executed against an installed environment, they can use the virtualenv created by onslaught after it runs.

What do you think of that scope? If we add in specialized tests, I fear this codebase would grow too complex and the test criteria would become hard to remember. If we allowed plugins, it would become too configurable, and then if you discover a codebase passes an onslaught run, you'll be less certain about what that means.

I like check-miscaptures.py, and it seems like the kind of test I do want to be included as part of the onslaught criteria. User code might intend to use that gotcha behavior, but authors may also intend to violate PEP8 style or pyflakes (such as by from foo import *). My idea for onslaught is to not support such variation.

However, I'd prefer check-miscaptures.py to be a pyflakes feature! Have you proposed it to them? I wonder if they have an issue with pyflakes becoming more strict over time. We should find out if we're going to define onslaught criteria in terms of pyflakes.

nejucomo commented 9 years ago

Note: there's a situation somewhat similar to check-miscaptures.py versus pyflakes in this codebase already: I added check_sdist_log.py to compensate for the setuptools looseness in setup.py sdist which exits successfully even after issuing warnings.

If onslaught ruled the world, then setuptools would be modified to treat all such warnings as errors and exit with an error. That would probably be unacceptable because it would break many packages. Perhaps we could convince setuptools (or better distutils) to add a --strict option to the sdist command (or all commands!). This might be the best compromise from onslaught's perspective because it could remove this log checker and just pass that flag.

daira commented 9 years ago

Adding check-miscaptures to pyflakes is not straightforward because, for both performance and complexity reasons, you'd want to integrate it with the traversal that pyflakes already does. And I don't really have time to dig into that code (i.e. https://github.com/pyflakes/pyflakes/blob/master/pyflakes/checker.py).

nejucomo commented 9 years ago

It sounds like there are three "development" reasons not to add a check-miscaptures feature to pyflakes:

These all seem surmountable, given enough time. The feature seems in scope for pyflakes at a glance, so I'd like to submit a ticket with them to see if that project accepts the feature idea. Do you mind if I file a ticket with them, or would you like to?

daira commented 9 years ago

I'm fine with you submitting a ticket; please link it here so that I can comment on it.