Process doctest scope directly under the module scope

[jayvdb]

Explicitly place the doctest directly under the module scope,
instead of processing it while within the scope where the
docstring was encountered.

The primary benefit of moving the doctest scope directly under
the module scope is that it may efficiently be found, as it
may only appear second in the stack.

However it also ensures that the doctest scope can not access
names in scopes between the module scope and the object where the
docstring was encountered. As it was, class scope rules prevented
the doctest scope from accessing names in a class scope, however
the doctest scope was able to access names in an outer function,
if the doctest appeared in a nested function.
Note that there was no real bug there, as the doctest module does
not process doctest in nested functions (Python issue #1650090),
so doctest appearing in nested functions are informational only.
pyflakes does inspect doctest in nested functions, and now it
ensures they can not access names in an outer function.

[ryneeverett]

Shouldn't the inner_function be accessible in its own doctest? It no longer is now that the doctest is in global scope. That is, the following would now raise UndefinedName:

        self.flakes("""
        def doctest_stuff():
            def inner_function():
                '''
                    >>> inner_function()
                '''
                return 1
            m = inner_function()
            return m
        """)

[jayvdb]

I agree it would be useful if inner_function was included in the doctest scope, and that should discussed on https://bugs.python.org/issue1650090 . It is easy on the pyflakes side, to add the name into the DoctestScope.

Maybe we shouldnt process the doctest in nested functions, as you've pointed out there is a chance that this changeset would cause new errors. And those new errors couldnt easily be resolved, as we dont know yet how doctest in nested functions will be implemented.

[ryneeverett]

Maybe we shouldnt process the doctest in nested functions

Makes sense to me. There might even be a case for adding an error message for nested doctests since they wont run.

[jayvdb]

Maybe we shouldnt process the doctest in nested functions

Makes sense to me.

I've implemented that, and added tests.

There might even be a case for adding an error message for nested doctests since they wont run.

That is easy to add, if @bitglue agrees it is OK.

[ryneeverett]

Would a simpler test suffice? For example a would-be SyntaxError:

    self.flakes('''
    def doctest_stuff():
        def inner_function():
            """
                >>> if True:
                ... pass
            """
    ''')

[jayvdb]

A simpler test would be good. I'd go with a slightly more obvious syntax error, which doesnt use doctest indentation, like syntax error or if: pass. Is there a more brutally obvious syntax error?

I'd prefer to keep the existing use of 'inner_function' and 'm', so that if/when nested functions are processed by doctest, those variables will be re-considered, if only because git blame will point the coder back to this discussion. But I'm happy to remove them.

[ryneeverett]

A simpler test would be good. I'd go with a slightly more obvious syntax error, which doesnt use doctest indentation, like syntax error or if: pass. Is there a more brutally obvious syntax error?

I like syntax error myself.

I'd prefer to keep the existing use of 'inner_function' and 'm', so that if/when nested functions are processed by doctest, those variables will be re-considered, if only because git blame will point the coder back to this discussion. But I'm happy to remove them.

I don't really care as long as the error is more obvious.

[jayvdb]

I've added 'syntax error', and a more detailed comment.

[ryneeverett]

There might even be a case for adding an error message for nested doctests since they wont run.

That is easy to add, if @bitglue agrees it is OK.

I'm having second thoughts. I guess I was initially thinking we'd have to skip them with an if: pass and if we're doing that we might as well raise an error. But you did it a better way and I'm not sure adding the code is worth the maintenance.