Lint with flake8-bugbear (#10602)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

setup.cfg

@@ -11,7 +11,7 @@ upload = upload --sign --identity=36580288
 
 [flake8]
 max-line-length = 95
-ignore = E116,E241,E251,E741,W504,I101
+ignore = E116,E241,E251,E741,W504,I101,B006,B023
 exclude = .git,.tox,.venv,tests/roots/*,build/*,doc/_build/*,sphinx/search/*,doc/usage/extensions/example*.py
 application-import-names = sphinx
 import-order-style = smarkets

setup.py

@@ -41,6 +41,7 @@
     'lint': [
         'flake8>=3.5.0',
         'flake8-comprehensions',
+        'flake8-bugbear',
         'isort',
         'mypy>=0.950',
         'sphinx-lint',

sphinx/builders/linkcheck.py

@@ -301,7 +301,7 @@ def check_uri() -> Tuple[str, str, int]:
                 req_url = encode_uri(req_url)
 
             # Get auth info, if any
-            for pattern, auth_info in self.auth:
+            for pattern, auth_info in self.auth:  # noqa: B007 (false positive)
                 if pattern.match(uri):
                     break
             else:

sphinx/domains/c.py

@@ -197,11 +197,11 @@ def describe_signature(self, signode: TextElement, mode: str,
         # just print the name part, with template args, not template params
         if mode == 'noneIsName':
             if self.rooted:
-                assert False, "Can this happen?"  # TODO
+                raise AssertionError("Can this happen?")  # TODO
                 signode += nodes.Text('.')
             for i in range(len(self.names)):
                 if i != 0:
-                    assert False, "Can this happen?"  # TODO
+                    raise AssertionError("Can this happen?")  # TODO
                     signode += nodes.Text('.')
                 n = self.names[i]
                 n.describe_signature(signode, mode, env, '', symbol)
@@ -1488,7 +1488,7 @@ def describe_signature(self, signode: TextElement, mode: str,
             mainDeclNode += addnodes.desc_sig_keyword(prefix, prefix)
             mainDeclNode += addnodes.desc_sig_space()
         else:
-            assert False
+            raise AssertionError()
         self.declaration.describe_signature(mainDeclNode, mode, env, self.symbol)
         if self.semicolon:
             mainDeclNode += addnodes.desc_sig_punctuation(';', ';')
@@ -1518,11 +1518,11 @@ class Symbol:
     debug_show_tree = False
 
     def __copy__(self):
-        assert False  # shouldn't happen
+        raise AssertionError()  # shouldn't happen
 
     def __deepcopy__(self, memo):
         if self.parent:
-            assert False  # shouldn't happen
+            raise AssertionError()  # shouldn't happen
         else:
             # the domain base class makes a copy of the initial data, which is fine
             return Symbol(None, None, None, None, None)
@@ -1543,7 +1543,7 @@ def _assert_invariants(self) -> None:
 
     def __setattr__(self, key: str, value: Any) -> None:
         if key == "children":
-            assert False
+            raise AssertionError()
         else:
             return super().__setattr__(key, value)
 
@@ -3091,7 +3091,7 @@ def parse_declaration(self, objectType: str, directiveType: str) -> ASTDeclarati
         elif objectType == 'type':
             declaration = self._parse_type(named=True, outer='type')
         else:
-            assert False
+            raise AssertionError()
         if objectType != 'macro':
             self.skip_ws()
             semicolon = self.skip_string(';')

sphinx/domains/cpp.py

@@ -754,15 +754,15 @@ def describe_signature(self, signode: TextElement, mode: str,
         # just print the name part, with template args, not template params
         if mode == 'noneIsName':
             if self.rooted:
-                assert False, "Can this happen?"  # TODO
+                raise AssertionError("Can this happen?")  # TODO
                 signode += nodes.Text('::')
             for i in range(len(self.names)):
                 if i != 0:
-                    assert False, "Can this happen?"  # TODO
+                    raise AssertionError("Can this happen?")  # TODO
                     signode += nodes.Text('::blah')
                 n = self.names[i]
                 if self.templates[i]:
-                    assert False, "Can this happen?"  # TODO
+                    raise AssertionError("Can this happen?")  # TODO
                     signode += nodes.Text("template")
                     signode += nodes.Text(" ")
                 n.describe_signature(signode, mode, env, '', symbol)
@@ -1386,7 +1386,7 @@ def _stringify(self, transform: StringifyTransform) -> str:
         if self.isNewTypeId:
             res.append(transform(self.typ))
         else:
-            assert False
+            raise AssertionError()
         if self.initList is not None:
             res.append(transform(self.initList))
         return ''.join(res)
@@ -1413,7 +1413,7 @@ def describe_signature(self, signode: TextElement, mode: str,
         if self.isNewTypeId:
             self.typ.describe_signature(signode, mode, env, symbol)
         else:
-            assert False
+            raise AssertionError()
         if self.initList is not None:
             self.initList.describe_signature(signode, mode, env, symbol)
 
@@ -1990,7 +1990,7 @@ def describe_signature(self, signode: TextElement, mode: str,
                 signode += addnodes.desc_sig_keyword('auto', 'auto')
                 signode += addnodes.desc_sig_punctuation(')', ')')
             else:
-                assert False, self.placeholderType
+                raise AssertionError(self.placeholderType)
 
 
 class ASTFunctionParameter(ASTBase):
@@ -3095,8 +3095,7 @@ def get_id(self, version: int, objectType: str = None,
                 elif objectType == 'type':  # just the name
                     res.append(symbol.get_full_nested_name().get_id(version))
                 else:
-                    print(objectType)
-                    assert False
+                    raise AssertionError(objectType)
             else:  # only type encoding
                 if self.decl.is_function_type():
                     raise NoOldIdError()
@@ -3125,8 +3124,7 @@ def get_id(self, version: int, objectType: str = None,
             elif objectType == 'type':  # just the name
                 res.append(symbol.get_full_nested_name().get_id(version))
             else:
-                print(objectType)
-                assert False
+                raise AssertionError(objectType)
         else:  # only type encoding
             # the 'returnType' of a non-function type is simply just the last
             # type, i.e., for 'int*' it is 'int'
@@ -4049,7 +4047,7 @@ def describe_signature(self, signode: desc_signature, mode: str,
             mainDeclNode += addnodes.desc_sig_keyword('enumerator', 'enumerator')
             mainDeclNode += addnodes.desc_sig_space()
         else:
-            assert False, self.objectType
+            raise AssertionError(self.objectType)
         self.declaration.describe_signature(mainDeclNode, mode, env, self.symbol)
         lastDeclNode = mainDeclNode
         if self.trailingRequiresClause:
@@ -4103,11 +4101,11 @@ class Symbol:
     debug_show_tree = False  # overridden by the corresponding config value
 
     def __copy__(self):
-        assert False  # shouldn't happen
+        raise AssertionError()  # shouldn't happen
 
     def __deepcopy__(self, memo):
         if self.parent:
-            assert False  # shouldn't happen
+            raise AssertionError()  # shouldn't happen
         else:
             # the domain base class makes a copy of the initial data, which is fine
             return Symbol(None, None, None, None, None, None, None)
@@ -4131,7 +4129,7 @@ def _assert_invariants(self) -> None:
 
     def __setattr__(self, key: str, value: Any) -> None:
         if key == "children":
-            assert False
+            raise AssertionError()
         else:
             return super().__setattr__(key, value)
 
@@ -5991,7 +5989,7 @@ def _parse_simple_type_specifiers(self) -> ASTTrailingTypeSpecFundamental:
             if modifier is not None:
                 self.fail("Can not have {} without a floating point type.".format(modifier))
         else:
-            assert False, "Unhandled type {}".format(typ)
+            raise AssertionError("Unhandled type {}".format(typ))
 
         canonNames: List[str] = []
         if modifier is not None:
@@ -6513,7 +6511,7 @@ def _parse_type(self, named: Union[bool, str], outer: str = None) -> ASTType:
                 elif outer == 'function':
                     desc = "If the function has no return type"
                 else:
-                    assert False
+                    raise AssertionError()
                 prevErrors.append((exUntyped, desc))
                 self.pos = startPos
                 try:
@@ -6526,7 +6524,7 @@ def _parse_type(self, named: Union[bool, str], outer: str = None) -> ASTType:
                     elif outer == 'function':
                         desc = "If the function has a return type"
                     else:
-                        assert False
+                        raise AssertionError()
                     prevErrors.append((exTyped, desc))
                     # Retain the else branch for easier debugging.
                     # TODO: it would be nice to save the previous stacktrace
@@ -6538,7 +6536,7 @@ def _parse_type(self, named: Union[bool, str], outer: str = None) -> ASTType:
                         elif outer == 'function':
                             header = "Error when parsing function declaration."
                         else:
-                            assert False
+                            raise AssertionError()
                         raise self._make_multi_error(prevErrors, header) from exTyped
                     else:
                         # For testing purposes.
@@ -6997,7 +6995,7 @@ def parse_declaration(self, objectType: str, directiveType: str) -> ASTDeclarati
         elif objectType == 'enumerator':
             declaration = self._parse_enumerator()
         else:
-            assert False
+            raise AssertionError()
         templatePrefix = self._check_template_consistency(declaration.name,
                                                           templatePrefix,
                                                           fullSpecShorthand=False,
@@ -7924,7 +7922,7 @@ def checkType() -> bool:
             if objtypes:
                 return declTyp in objtypes
             print("Type is %s, declaration type is %s" % (typ, declTyp))
-            assert False
+            raise AssertionError()
         if not checkType():
             logger.warning("cpp:%s targets a %s (%s).",
                            typ, s.declaration.objectType,

sphinx/testing/util.py

@@ -24,17 +24,17 @@
 
 def assert_re_search(regex: Pattern, text: str, flags: int = 0) -> None:
     if not re.search(regex, text, flags):
-        assert False, '%r did not match %r' % (regex, text)
+        raise AssertionError('%r did not match %r' % (regex, text))
 
 
 def assert_not_re_search(regex: Pattern, text: str, flags: int = 0) -> None:
     if re.search(regex, text, flags):
-        assert False, '%r did match %r' % (regex, text)
+        raise AssertionError('%r did match %r' % (regex, text))
 
 
 def assert_startswith(thing: str, prefix: str) -> None:
     if not thing.startswith(prefix):
-        assert False, '%r does not start with %r' % (thing, prefix)
+        raise AssertionError('%r does not start with %r' % (thing, prefix))
 
 
 def assert_node(node: Node, cls: Any = None, xpath: str = "", **kwargs: Any) -> None:

tests/test_build_epub.py

@@ -381,4 +381,4 @@ def test_run_epubcheck(app):
         except CalledProcessError as exc:
             print(exc.stdout.decode('utf-8'))
             print(exc.stderr.decode('utf-8'))
-            assert False, 'epubcheck exited with return code %s' % exc.returncode
+            raise AssertionError('epubcheck exited with return code %s' % exc.returncode)

tests/test_build_gettext.py

@@ -60,7 +60,7 @@ def test_msgfmt(app):
         except CalledProcessError as exc:
             print(exc.stdout)
             print(exc.stderr)
-            assert False, 'msginit exited with return code %s' % exc.returncode
+            raise AssertionError('msginit exited with return code %s' % exc.returncode)
 
         assert (app.outdir / 'en_US.po').isfile(), 'msginit failed'
         try:
@@ -72,7 +72,7 @@ def test_msgfmt(app):
         except CalledProcessError as exc:
             print(exc.stdout)
             print(exc.stderr)
-            assert False, 'msgfmt exited with return code %s' % exc.returncode
+            raise AssertionError('msgfmt exited with return code %s' % exc.returncode)
 
         mo = app.outdir / 'en' / 'LC_MESSAGES' / 'test_root.mo'
         assert mo.isfile(), 'msgfmt failed'

tests/test_build_html.py

@@ -79,7 +79,7 @@ def checker(nodes):
         for node in nodes:
             if node.tail and rex.search(node.tail):
                 return True
-        assert False, '%r not found in tail of any nodes %s' % (check, nodes)
+        raise AssertionError('%r not found in tail of any nodes %s' % (check, nodes))
     return checker
 
 
@@ -114,9 +114,9 @@ def get_text(node):
             if all(not rex.search(get_text(node)) for node in nodes):
                 return
 
-        assert False, ('%r not found in any node matching '
-                       'path %s in %s: %r' % (check, path, fname,
-                                              [node.text for node in nodes]))
+        raise AssertionError(('%r not found in any node matching '
+                              'path %s in %s: %r' % (check, path, fname,
+                                                     [node.text for node in nodes])))
 
 
 @pytest.mark.sphinx('html', testroot='warnings')
@@ -1520,7 +1520,7 @@ def test_html_math_renderer_is_duplicated(make_app, app_params):
     try:
         args, kwargs = app_params
         make_app(*args, **kwargs)
-        assert False
+        raise AssertionError()
     except ConfigError as exc:
         assert str(exc) == ('Many math_renderers are registered. '
                             'But no math_renderer is selected.')
@@ -1550,7 +1550,7 @@ def test_html_math_renderer_is_mismatched(make_app, app_params):
     try:
         args, kwargs = app_params
         make_app(*args, **kwargs)
-        assert False
+        raise AssertionError()
     except ConfigError as exc:
         assert str(exc) == "Unknown math_renderer 'imgmath' is given."
 

tests/test_build_latex.py

@@ -60,8 +60,8 @@ def compile_latex_document(app, filename='python.tex'):
     except CalledProcessError as exc:
         print(exc.stdout)
         print(exc.stderr)
-        assert False, '%s exited with return code %s' % (app.config.latex_engine,
-                                                         exc.returncode)
+        raise AssertionError('%s exited with return code %s' % (app.config.latex_engine,
+                                                                exc.returncode))
 
 
 def skip_if_requested(testfunc):
@@ -992,7 +992,7 @@ def test_image_in_section(app, status, warning):
 def test_latex_logo_if_not_found(app, status, warning):
     try:
         app.builder.build_all()
-        assert False  # SphinxError not raised
+        raise AssertionError()  # SphinxError not raised
     except Exception as exc:
         assert isinstance(exc, SphinxError)
 

tests/test_build_texinfo.py

@@ -55,7 +55,7 @@ def test_texinfo(app, status, warning):
     except CalledProcessError as exc:
         print(exc.stdout)
         print(exc.stderr)
-        assert False, 'makeinfo exited with return code %s' % exc.retcode
+        raise AssertionError('makeinfo exited with return code %s' % exc.retcode)
 
 
 @pytest.mark.sphinx('texinfo', testroot='markup-rubric')

tests/test_config.py

@@ -48,13 +48,13 @@ def test_core_config(app, status, warning):
 
     # invalid values
     with pytest.raises(AttributeError):
-        getattr(cfg, '_value')
+        cfg._value
     with pytest.raises(AttributeError):
-        getattr(cfg, 'nonexisting_value')
+        cfg.nonexisting_value
 
     # non-value attributes are deleted from the namespace
     with pytest.raises(AttributeError):
-        getattr(cfg, 'sys')
+        cfg.sys
 
     # setting attributes
     cfg.project = 'Foo'

tests/test_domain_cpp.py

@@ -1140,7 +1140,7 @@ def check(spec, text, file):
         res = re.search(pattern, text)
         if not res:
             print("Pattern\n\t%s\nnot found in %s" % (pattern, file))
-            assert False
+            raise AssertionError()
     rolePatterns = [
         ('', 'Sphinx'),
         ('', 'Sphinx::version'),
@@ -1181,7 +1181,7 @@ def check(spec, text, file):
         res = re.search(pattern, text)
         if not res:
             print("Pattern\n\t%s\nnot found in %s" % (pattern, file))
-            assert False
+            raise AssertionError()
     rolePatterns = [
         ('', 'Sphinx'),
         ('', 'Sphinx::version'),

tests/test_ext_doctest.py

@@ -18,7 +18,7 @@ def test_build(app, status, warning):
     cleanup_called = 0
     app.builder.build_all()
     if app.statuscode != 0:
-        assert False, 'failures in doctests:' + status.getvalue()
+        raise AssertionError('failures in doctests:' + status.getvalue())
     # in doctest.txt, there are two named groups and the default group,
     # so the cleanup function must be called three times
     assert cleanup_called == 3, 'testcleanup did not get executed enough times'
@@ -88,7 +88,7 @@ def test_skipif(app, status, warning):
     recorded_calls = Counter()
     app.builder.build_all()
     if app.statuscode != 0:
-        assert False, 'failures in doctests:' + status.getvalue()
+        raise AssertionError('failures in doctests:' + status.getvalue())
     # The `:skipif:` expressions are always run.
     # Actual tests and setup/cleanup code is only run if the `:skipif:`
     # expression evaluates to a False value.