Should nested functions be considered public?

[MarkKoz]

Currently nested functions are considered public since they can trigger e.g. ANN201. Of course such errors could be circumvented by prefixing the name with an underscore or with a noqa. However, nested functions are only within the scope of the parent function, so they aren't necessarily public despite their naming convention. Conceptually, they could be considered public if they are returned and used elsewhere.

The area in which this issue often comes up is in the implementation of decorators. It is often redundant to annotate the nested wrapper function because it uses functools.wraps to carry over annotations. There are exceptions in cases of greater complexity. For example, some decorators wrap the return value in a consistent object or explicitly use certain arguments so the annotations can be an aid in explaining the nearby code.

Do you think the plugin should treat nested functions differently in this regard, or is it fair and not much of an inconvenience to just prefix their names with an underscore? Would this be a difficult feature to implement? Should it be opt-in behaviour or even have a separate set of errors?

[sco1]

Sorry I missed this, it doesn't seem like discussions have webhooks implemented.

Do you think the plugin should treat nested functions differently in this regard, or is it fair and not much of an inconvenience to just prefix their names with an underscore?

In my opinion this is fair in general, but the decorator definition case presents a reasonable scenario where annotations might not add value and you'd have to noqa or underscore out perfectly idiomatic code just to appease the linter. I think this is a reasonable case for the potential of some opinionated handling.

Would this be a difficult feature to implement? Should it be opt-in behaviour or even have a separate set of errors?

Difficulty depends. Conceptually, it would be great to be able to explicitly special case decorator definitions but there's no great way to reliably identify these statically and within the per-file context of flake8. Something similar to #99 might be beneficial if functools.wraps is explicitly used. Would that be good enough?

[MarkKoz]

While the idea of #99 would be a step in the right direction, it doesn't cover other common cases. Continuing with the decorators example:

• Decorators which take arguments are implemented with yet another nested function, but these don't use functools.wraps of course.
• The decorator function itself, rather than the wrapper, is not nested (except for the case mentioned just above). It often isn't useful to annotate its return annotation and the callable argument. However, I don't see a reliable way to distinguish it from any other function.

This may be broadening the scope of the original issue. At the time, I had not realised that even if the function is named so that it's protected, it'd still be affected by ANN001 and ANN201. In any case, nested functions are still distinct enough from protected functions to lump them in the same group. There currently isn't a good compromise - either ignore those warnings for non-nested functions too or add noqas to every nested function defined. Here are various ideas:

1. Add ANN0xx-like and ANN2xx-like warnings that only apply to nested functions.
   • Upside is flexibility. Downside is adding 4 new warnings.
2. Add a flag to disable ANN0xx and ANN2xx (or just all) warnings for all nested functions.
3. Add something like allow-untyped-defs which only applies to nested functions.
   • This is the idea I'm liking the most. It makes the annotations opt-in which, in my experience, would be less common than opting out for nested functions. The opposite is true for non-nested, with which a completely unannotated function is more likely the result of forgetfulness than intent.

Presumably it is feasible to use AST to detect if a function is nested.

[sco1]

I like option 3 the best & it should be pretty straightforward to implement; we're already keeping track of context when walking the AST and doing almost exactly the same thing for identifying class methods.

I don't think I'll be able to work on this until after the New Year but it should be a fairly quick release once I have a bit of time.

[sco1]

#104 Should be good to go if you're interested in trying it out.

I'll give it a couple days to simmer so I can give it a fresh look before cutting the release.