Repository: sphinx-doc/sphinx

Issue:
sphinx.util.get_module_source fails for builtins
Subject: Importing from builtins (or itertools) forces the module file to be marked as *changed*


### Problem
Importing itertools or builtins in a module will always mark this module to be changed when using the `BuildEnvironment.get_outdated_files` method. This then always triggers a rebuilt of the module, slowing down the build.

#### Procedure to reproduce the problem
##### Quick demonstration or the bug source
The source of the problem lies in [sphinx.util.get_module_source](https://github.com/sphinx-doc/sphinx/blob/master/sphinx/util/__init__.py#L305), you can run

```python
from sphinx.util import get_module_source
print(get_module_source('builtins'), get_module_source('itertools'))
# ('string', None) ('string', None)
```
which essentially is because builtins (and itertools) do not have an associated `__file__` attribute, but a `__loader__`, and you get

```python
import builtins
print(builtins.__loader__.get_source('builtins'))
# None
```
The problem here is, that `get_module_source` returns `'string'`.

##### Example project
To see the result of this, consider the following small project. We create a documentation of the `conf.py` module using the `automodule` directive:

`conf.py`
```python
from itertools import chain
import sys
sys.path.insert(0, '.')

extensions = ['sphinx.ext.autodoc']
```

`contents.rst`
```rst
API Reference
=============

.. automodule:: conf
    :members:

.. toctree::

```
and build it with

```bash
sphinx-build . html -b html -d doctrees
sphinx-build . latex -b latex -d doctrees
```

#### Error logs / results
<details>
<summary>The first build goes fine, and 1 source file is added.</summary>

```
Running Sphinx v1.8.2
making output directory...
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 1 source files that are out of date
updating environment: 1 added, 0 changed, 0 removed
reading sources... [100%] contents                                                                                                                                                                                                                                              
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] contents                                                                                                                                                                                                                                               
generating indices... genindex py-modindex
writing additional pages... search
copying static files... done
copying extra files... done
dumping search index in English (code: en) ... done
dumping object inventory... done
build succeeded.

The HTML pages are in html.

```
</details>

<details>
<summary>The second build marks one file as changed.</summary>

```
Running Sphinx v1.8.2
making output directory...
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
building [latex]: all documents
updating environment: [] 0 added, 1 changed, 0 removed
reading sources... [100%] contents                                                                                                                                                                                                                                              
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
processing Python.tex...contents 
resolving references...
writing... done
copying TeX support files...
done
build succeeded.

The LaTeX files are in latex.
Run 'make' in that directory to run these through (pdf)latex
(use `make latexpdf' here to do that automatically).
```
</details>

It says `updating environment: [] 0 added, 1 changed, 0 removed` (which is clearly wrong since none of the files has been changed). For larger projects however, this means that this the documented module is always rebuilt which slows down the built process.

I traced the problem down:

1. The `AutodocDirective` generates a ModuleDocumenter (see [AutodocDirective.run](https://github.com/sphinx-doc/sphinx/blob/master/sphinx/ext/autodoc/directive.py#L130))
2. This then checks the dependencies of the module (in this case, itertools) in it's generate method (see [Documenter.generate](https://github.com/sphinx-doc/sphinx/blob/master/sphinx/ext/autodoc/__init__.py#L726))
3. The created `ModuleAnalyzer` uses `get_module_source` to investigate `itertools` (see [ModuleAnalyzer.for_module](https://github.com/sphinx-doc/sphinx/blob/master/sphinx/pycode/__init__.py#L76))
4. Because of the returned `'string'` by `get_module_source` (see [sphinx.util.get_module_source](https://github.com/sphinx-doc/sphinx/blob/master/sphinx/util/__init__.py#L305)), the `filename_set` get's a `'<string>'` added
5. And the `AutodocDirective` adds this to the dependencies in (see [AutodocDirective.run](https://github.com/sphinx-doc/sphinx/blob/master/sphinx/ext/autodoc/directive.py#L140))
6. Finally, when rebuilding the docs, the `get_outdated_files` method of the `BuildEnvironment` then looks for a file name called `path.join(self.srcdir, '<string>')` (see [BuildEnvironment.get_outdated_files](https://github.com/sphinx-doc/sphinx/blob/master/sphinx/environment/__init__.py#L452)) which is clearly not existing and hence marks the original module (`conf.py`) as *changed*.

#### Expected results
The builtins module has to be better handled when it comes to the dependencies in a module.

### Reproducible project / your project
(see the `conf.py` and `contents.rst` shown above)
[example.zip](https://github.com/sphinx-doc/sphinx/files/2569719/example.zip)


### Environment info
- OS: macOS High Sierra, Version 10.13.6
- Python version: 3.7.1 (default, Oct 23 2018, 14:07:42) [Clang 4.0.1 (tags/RELEASE_401/final)]
- Sphinx version: 1.8.2


Discussion:

Patch:
diff --git a/CHANGES b/CHANGES
index b5d984306..929b468fd 100644
--- a/CHANGES
+++ b/CHANGES
@@ -17,6 +17,7 @@ Bugs fixed
 ----------
 
 * #5460: html search does not work with some 3rd party themes
+* #5614: autodoc: incremental build is broken when builtin modules are imported
 
 Testing
 --------
diff --git a/sphinx/util/__init__.py b/sphinx/util/__init__.py
index 109b098ce..9bc88c82f 100644
--- a/sphinx/util/__init__.py
+++ b/sphinx/util/__init__.py
@@ -301,7 +301,9 @@ def get_module_source(modname):
             raise PycodeError('error getting filename for %r' % filename, err)
     if filename is None and loader:
         try:
-            return 'string', loader.get_source(modname)
+            filename = loader.get_source(modname)
+            if filename:
+                return 'string', filename
         except Exception as err:
             raise PycodeError('error getting source for %r' % modname, err)
     if filename is None:


Test patch:
diff --git a/tests/test_util.py b/tests/test_util.py
index 8e4fbd6b0..a3745e7a1 100644
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -12,9 +12,14 @@
 import pytest
 from mock import patch
 
+from six import PY2
+
+import sphinx
+from sphinx.errors import PycodeError
 from sphinx.testing.util import strip_escseq
 from sphinx.util import (
-    display_chunk, encode_uri, parselinenos, status_iterator, xmlname_checker
+    display_chunk, encode_uri, get_module_source, parselinenos, status_iterator,
+    xmlname_checker
 )
 from sphinx.util import logging
 
@@ -42,6 +47,19 @@ def test_display_chunk():
     assert display_chunk(('hello', 'sphinx', 'world')) == 'hello .. world'
 
 
+def test_get_module_source():
+    if PY2:
+        assert get_module_source('sphinx') == ('file', sphinx.__file__.replace('.pyc', '.py'))
+    else:
+        assert get_module_source('sphinx') == ('file', sphinx.__file__)
+
+    # failed to obtain source information from builtin modules
+    with pytest.raises(PycodeError):
+        get_module_source('builtins')
+    with pytest.raises(PycodeError):
+        get_module_source('itertools')
+
+
 @pytest.mark.sphinx('dummy')
 @patch('sphinx.util.console._tw', 40)  # terminal width = 40
 def test_status_iterator(app, status, warning):