support for "local plugins" (configured explicitly instead of via entry points)

We have a number of repo-specific lint rules we'd like to apply. It's important that engineers working in the repo can add or update lint rules with low friction, so we want the plugin code in the same repo that is being checked. It's possible to achieve this by adding a setup.py to a flake8 plugin subdirectory and modifying requirements files to separately editable-install it, but it's a bit unpleasant: generates egg-info metadata that has to be committed or ignored, takes extra time on pip install because editable installs are always reinstalled, requires a full directory with at least three files in it, where otherwise you'd probably just need a single module.

It would be really nice to have a way to configure a plugin purely in setup.cfg, without having to involve setuptools at all. Pytest is an example of a project that uses setuptools entry points, but also provides a way to configure local plugins without the use of setuptools.

This makes sense. I did some research and I think we could even translate the typical plugin language to these "local" plugins because of how much we rely on that syntax. Take for example the following:

>>> import pkg_resources
>>> pkg_resources.EntryPoint.parse('F = flake8.main.cli:main')
EntryPoint.parse('F = flake8.main.cli:main')
>>> ep = _
>>> ep.resolve()
<function main at 0x7f0587df2510>

I did this simply an environment where flake8 itself was installed. There was no entry-point otherwise registered for that.

We could utilize the same syntax (e.g., ViolationClass = module.path.to:Plugin) in our tox.ini/setup.cfg and allow for

added feature:accepted priority:medium labels

changed milestone to %3.x.0

Great! I'm working on an implementation.

Some thoughts as I work on this, in case you have comments on the design direction:

1. Since plugins can themselves provide new config options, we will need two-phase config parsing: first look for plugins in config files, then load all plugins and let them register options, then parse the rest of the config. I'll need to do a bit of refactoring in config.py to make this possible without wastefully double-reading the same config files.
2. I'm leaning towards putting plugins in a separate config-file section from other options (e.g. if our program_name is flake8 I'd look for plugins in a [flake8:plugins] section. There are two reasons for this. First, because it lets us use the elegant X = path.to:Plugin syntax with no worries about namespace collision with regular options. And second, it makes two-phase config parsing a bit cleaner, since there's a clean division of what section gets parsed in which phase; we don't have to read-and-ignore some options in either phase.

Let me know if any of this seems misguided to you! Thanks.

Actually, having written that out, I'm backtracking on point 2. I think it'll be simpler to just have a multi-line plugins key in the existing [flake8] section. Can still use the nice syntax, e.g.:

[flake8]
plugins =
  X = path.to:Plugin

And clean separation isn't really an issue either, we can just do a single .get(self.program_name, 'plugins') in first phase, and just ignore plugins option in second phase.

Some thoughts since I'm sensitive to wide-spreading pull requests that do way too much and end up breaking end-users.

1. Would it be terrible if we used a separate config file that Flake8 can detect at start-up and read ahead of time instead of trying to parse the same file(s) more than once?

2. If 1 is terrible, I'd rather be explicit. Instead of plugins = I'd rather use local_plugins.

3. I want syntax closer to your second example, specifically:

[section-title]
local_plugins =
    X = path.to:Plugin

Only because ini parsers can parse this file with =s so if we instead did:

[flake8:local_plugins]
X = path.to:Plugin
Y = path.to.another:Plugin

Then the way we handle these values would be gross and we have to reconstruct the entry-point string or do our own messy import hacking and EntryPoint faking.

I'm OK with a separate config file if you prefer that. It seems slightly nicer if you don't have to create a separate file, but either way works. I don't think the config modifications to avoid double-parsing will be too drastic. If we do a separate file, got any preference for the name?

Re syntax, yeah I'm definitely going with the multi-line version you prefer, it's better.

Re key-name, there are actually three types of plugins. I was going to go with extension_plugins, listen_plugins, and report_plugins to sort of match the entry point namespaces. I can prepend each of those with local_ if you like. This is a bikeshed I'm more than happy for you to paint, just let me know which color :)

oh geez. I never considered people might want to have local extension and report plugins. sigh As for listener plugins, I think that'll have to die since we're likely going to point people at yapf for auto-fixing violations.

I hate config file sprawl, so let's see what you come up with. As for explicit-ness, what if we do something like this:

[flake8:local-plugins]
extension =
    X = path.to:Plugin,
    Y = path.to.another:Plugin
report =
    msgpack = path.to.msgpack:Reporter,
    yolo = path.to.yolo:Reporter

Given that extension isn't intuitive, I'd also be open to that being checker but I see the parity you're working towards.

mentioned in merge request !197 (merged)

Ok, I submitted a first draft at !197 (merged). The most invasive change required is not actually caching in ConfigFileFinder; that in itself is pretty simple. The hard part is that we need args in order to find local config files, and we can't add support for local plugins if we can't find local config files pre-plugin-loading. So I had to surface ConfigFileFinder as a first-class citizen in Application, instantiated using preliminary args from parse_known_args, and used both for finding local plugins and for finding the main config.

This change could be made less invasive if we didn't worry about double finding and reading of config files, because then we wouldn't need to reuse a ConfigFileFinder. But I'm not sure using two separate ConfigFileFinder is a good direction, both for startup speed reasons and because I think it'll be really confusing if config files found for local plugin loading can be inconsistent from config files found for main config.

The key question here is, could args found from preliminary CLI parsing ever be different from args found in main CLI parsing. I can't think why they would be -- the only difference between the two should be skippage of unknown options. But if they ever are, that could lead to a change in behavior in config file finding here.

Anyway, take a look and let me know if you see a better way to do this. Thanks for the thoughts and advice today!

changed milestone to %3.5.0

closed via merge request !197 (merged)