Merge pull request #3241 from jon-stumpf/jss-updates

Add pre-commit config for flake8

.pre-commit-config.yaml

@@ -3,3 +3,14 @@ repos:
     rev: 19.3b0
     hooks:
       - id: black
+  - repo: https://gitlab.com/pycqa/flake8
+    rev: 3.7.7
+    hooks:
+      - id: flake8
+        additional_dependencies: [flake8-bugbear]
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v2.1.0
+    hooks:
+      - id: check-byte-order-marker
+      - id: trailing-whitespace
+      - id: end-of-file-fixer

docs/Makefile

@@ -17,4 +17,4 @@ help:
 # Catch-all target: route all unknown targets to Sphinx using the new
 # "make mode" option.  $(O) is meant as a shortcut for $(SPHINXOPTS).
 %: Makefile
-	@$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
+	@$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)

docs/flaskstyle.sty

@@ -9,7 +9,7 @@
   \begin{titlepage}%
     \let\footnotesize\small
     \let\footnoterule\relax
-    % Apply following fix only on PDF output, i.e. pdfoutput macro is not 
+    % Apply following fix only on PDF output, i.e. pdfoutput macro is not
     % undefined
     \ifx\pdfoutput\undefined\else
       \begingroup

docs/patterns/appfactories.rst

@@ -93,13 +93,13 @@ To run such an application, you can use the :command:`flask` command::
 
     $ export FLASK_APP=myapp
     $ flask run
-    
-Flask will automatically detect the factory (``create_app`` or ``make_app``) 
+
+Flask will automatically detect the factory (``create_app`` or ``make_app``)
 in ``myapp``. You can also pass arguments to the factory like this::
 
     $ export FLASK_APP="myapp:create_app('dev')"
     $ flask run
-    
+
 Then the ``create_app`` factory in ``myapp`` is called with the string
 ``'dev'`` as the argument. See :doc:`/cli` for more detail.
 

docs/testing.rst

@@ -54,7 +54,7 @@ the application for testing and initializes a new database::
     def client():
         db_fd, flaskr.app.config['DATABASE'] = tempfile.mkstemp()
         flaskr.app.config['TESTING'] = True
-        
+
         with flaskr.app.test_client() as client:
             with flaskr.app.app_context():
                 flaskr.init_db()

flask/_compat.py

@@ -64,7 +64,7 @@ def with_metaclass(meta, *bases):
     # dummy metaclass for one level of class instantiation that replaces
     # itself with the actual metaclass.
     class metaclass(type):
-        def __new__(cls, name, this_bases, d):
+        def __new__(mcs, name, this_bases, d):
             return meta(name, bases, d)
 
     return type.__new__(metaclass, "temporary_class", (), {})
@@ -97,7 +97,7 @@ if hasattr(sys, "pypy_version_info"):
         try:
             with _Mgr():
                 raise AssertionError()
-        except:
+        except:  # noqa: B001
             # We intentionally use a bare except here. See the comment above
             # regarding a pypy bug as to why.
             raise

flask/app.py

@@ -1961,7 +1961,7 @@ class Flask(_PackageBoundObject):
                 adapter.match(method="--")
             except MethodNotAllowed as e:
                 methods = e.valid_methods
-            except HTTPException as e:
+            except HTTPException:
                 pass
         rv = self.response_class()
         rv.allow.update(methods)
@@ -2399,7 +2399,7 @@ class Flask(_PackageBoundObject):
             except Exception as e:
                 error = e
                 response = self.handle_exception(e)
-            except:
+            except:  # noqa: B001
                 error = sys.exc_info()[1]
                 raise
             return response(environ, start_response)

flask/cli.py

@@ -230,7 +230,7 @@ def prepare_import(path):
 
 
 def locate_app(script_info, module_name, app_name, raise_if_not_found=True):
-    __traceback_hide__ = True
+    __traceback_hide__ = True  # noqa: F841
 
     try:
         __import__(module_name)
@@ -302,7 +302,7 @@ class DispatchingApp(object):
 
     def _load_in_background(self):
         def _load_app():
-            __traceback_hide__ = True
+            __traceback_hide__ = True  # noqa: F841
             with self._lock:
                 try:
                     self._load_unlocked()
@@ -313,20 +313,20 @@ class DispatchingApp(object):
         t.start()
 
     def _flush_bg_loading_exception(self):
-        __traceback_hide__ = True
+        __traceback_hide__ = True  # noqa: F841
         exc_info = self._bg_loading_exc_info
         if exc_info is not None:
             self._bg_loading_exc_info = None
             reraise(*exc_info)
 
     def _load_unlocked(self):
-        __traceback_hide__ = True
+        __traceback_hide__ = True  # noqa: F841
         self._app = rv = self.loader()
         self._bg_loading_exc_info = None
         return rv
 
     def __call__(self, environ, start_response):
-        __traceback_hide__ = True
+        __traceback_hide__ = True  # noqa: F841
         if self._app is not None:
             return self._app(environ, start_response)
         self._flush_bg_loading_exception()
@@ -364,7 +364,7 @@ class ScriptInfo(object):
         this multiple times will just result in the already loaded app to
         be returned.
         """
-        __traceback_hide__ = True
+        __traceback_hide__ = True  # noqa: F841
 
         if self._loaded_app is not None:
             return self._loaded_app
@@ -702,7 +702,7 @@ class CertParamType(click.ParamType):
 
             if value == "adhoc":
                 try:
-                    import OpenSSL
+                    import OpenSSL  # noqa: F401
                 except ImportError:
                     raise click.BadParameter(
                         "Using ad-hoc certificates requires pyOpenSSL.", ctx, param

flask/helpers.py

@@ -230,7 +230,7 @@ def url_for(endpoint, **values):
 
     For more information, head over to the :ref:`Quickstart <url-building>`.
 
-    Configuration values ``APPLICATION_ROOT`` and ``SERVER_NAME`` are only used when 
+    Configuration values ``APPLICATION_ROOT`` and ``SERVER_NAME`` are only used when
     generating URLs outside of a request context.
 
     To integrate applications, :class:`Flask` has a hook to intercept URL build
@@ -418,7 +418,7 @@ def flash(message, category="message"):
     )
 
 
-def get_flashed_messages(with_categories=False, category_filter=[]):
+def get_flashed_messages(with_categories=False, category_filter=()):
     """Pulls all flashed messages from the session and returns them.
     Further calls in the same request to the function will return
     the same messages.  By default just the messages are returned,

flask/templating.py

@@ -78,7 +78,7 @@ class DispatchingJinjaLoader(BaseLoader):
         raise TemplateNotFound(template)
 
     def _get_source_fast(self, environment, template):
-        for srcobj, loader in self._iter_loaders(template):
+        for _srcobj, loader in self._iter_loaders(template):
             try:
                 return loader.get_source(environment, template)
             except TemplateNotFound:

flask/testing.py

@@ -16,7 +16,7 @@ from contextlib import contextmanager
 
 from click.testing import CliRunner
 from flask.cli import ScriptInfo
-from werkzeug.test import Client, EnvironBuilder
+from werkzeug.test import Client
 from flask import _request_ctx_stack
 from flask.json import dumps as json_dumps
 from werkzeug.urls import url_parse

flask/wrappers.py

@@ -45,7 +45,8 @@ class Request(RequestBase, JSONMixin):
     #: a before/after handler (``request.url_rule.methods``) etc.
     #: Though if the request's method was invalid for the URL rule,
     #: the valid list is available in ``routing_exception.valid_methods``
-    #: instead (an attribute of the Werkzeug exception :exc:`~werkzeug.exceptions.MethodNotAllowed`)
+    #: instead (an attribute of the Werkzeug exception
+    #: :exc:`~werkzeug.exceptions.MethodNotAllowed`)
     #: because the request was never internally bound.
     #:
     #: .. versionadded:: 0.6

scripts/make-release.py

@@ -14,7 +14,7 @@ def parse_changelog():
     with open("CHANGES.rst") as f:
         lineiter = iter(f)
         for line in lineiter:
-            match = re.search("^Version\s+(.*)", line.strip())
+            match = re.search(r"^Version\s+(.*)", line.strip())
 
             if match is None:
                 continue

setup.cfg

@@ -22,3 +22,28 @@ source =
     flask
     .tox/*/lib/python*/site-packages/flask
     .tox/pypy/site-packages/flask
+
+[flake8]
+# B = bugbear
+# E = pycodestyle errors
+# F = flake8 pyflakes
+# W = pycodestyle warnings
+# B9 = bugbear opinions
+select = B, E, F, W, B9
+ignore =
+    # slice notation whitespace, invalid
+    E203
+    # import at top, too many circular import fixes
+    E402
+    # line length, handled by bugbear B950
+    E501
+    # bare except, handled by bugbear B001
+    E722
+    # bin op line break, invalid
+    W503
+# up to 88 allowed by bugbear B950
+max-line-length = 80
+per-file-ignores =
+    # __init__ modules export names
+    **/__init__.py: F401
+    **/_compat.py: E731, B301, F401

tests/templates/template_filter.html

@@ -1 +1 @@
-{{ value|super_reverse }}
+{{ value|super_reverse }}

tests/test_apps/helloworld/wsgi.py

@@ -1 +1 @@
-from hello import app
+from hello import app  # noqa: F401

tests/test_basic.py

@@ -163,7 +163,8 @@ def test_url_mapping(app, client):
     app.add_url_rule("/", "index", index)
     app.add_url_rule("/more", "more", more, methods=["GET", "POST"])
 
-    # Issue 1288: Test that automatic options are not added when non-uppercase 'options' in methods
+    # Issue 1288: Test that automatic options are not added
+    #             when non-uppercase 'options' in methods
     app.add_url_rule("/options", "options", options, methods=["options"])
 
     assert client.get("/").data == b"GET"
@@ -779,7 +780,7 @@ def test_teardown_request_handler_error(app, client):
         # exception.
         try:
             raise TypeError()
-        except:
+        except Exception:
             pass
 
     @app.teardown_request
@@ -791,7 +792,7 @@ def test_teardown_request_handler_error(app, client):
         # exception.
         try:
             raise TypeError()
-        except:
+        except Exception:
             pass
 
     @app.route("/")
@@ -963,7 +964,7 @@ def test_http_error_subclass_handling(app, client):
         return "banana"
 
     @app.errorhandler(403)
-    def handle_forbidden_subclass(e):
+    def handle_403(e):
         assert not isinstance(e, ForbiddenSubclass)
         assert isinstance(e, Forbidden)
         return "apple"
@@ -1065,12 +1066,12 @@ def test_error_handler_after_processor_error(app, client):
 
     @app.before_request
     def before_request():
-        if trigger == "before":
+        if _trigger == "before":
             1 // 0
 
     @app.after_request
     def after_request(response):
-        if trigger == "after":
+        if _trigger == "after":
             1 // 0
         return response
 
@@ -1082,7 +1083,7 @@ def test_error_handler_after_processor_error(app, client):
     def internal_server_error(e):
         return "Hello Server Error", 500
 
-    for trigger in "before", "after":
+    for _trigger in "before", "after":
         rv = client.get("/")
         assert rv.status_code == 500
         assert rv.data == b"Hello Server Error"
@@ -1459,10 +1460,12 @@ def test_static_route_with_host_matching():
     # Providing static_host without host_matching=True should error.
     with pytest.raises(Exception):
         flask.Flask(__name__, static_host="example.com")
-    # Providing host_matching=True with static_folder but without static_host should error.
+    # Providing host_matching=True with static_folder
+    # but without static_host should error.
     with pytest.raises(Exception):
         flask.Flask(__name__, host_matching=True)
-    # Providing host_matching=True without static_host but with static_folder=None should not error.
+    # Providing host_matching=True without static_host
+    # but with static_folder=None should not error.
     flask.Flask(__name__, host_matching=True, static_folder=None)
 
 
@@ -1574,12 +1577,12 @@ def test_max_content_length(app, client):
     @app.before_request
     def always_first():
         flask.request.form["myfile"]
-        assert False
+        AssertionError()
 
     @app.route("/accept", methods=["POST"])
     def accept_file():
         flask.request.form["myfile"]
-        assert False
+        AssertionError()
 
     @app.errorhandler(413)
     def catcher(error):
@@ -1765,7 +1768,7 @@ def test_preserve_only_once(app, client):
     def fail_func():
         1 // 0
 
-    for x in range(3):
+    for _x in range(3):
         with pytest.raises(ZeroDivisionError):
             client.get("/fail")
 

tests/test_blueprints.py

@@ -705,7 +705,7 @@ def test_add_template_test_with_name_and_template(app, client):
 def test_context_processing(app, client):
     answer_bp = flask.Blueprint("answer_bp", __name__)
 
-    template_string = lambda: flask.render_template_string(
+    template_string = lambda: flask.render_template_string(  # noqa: E731
         "{% if notanswer %}{{ notanswer }} is not the answer. {% endif %}"
         "{% if answer %}{{ answer }} is the answer.{% endif %}"
     )

tests/test_cli.py

@@ -56,7 +56,7 @@ def test_cli_name(test_apps):
 
 
 def test_find_best_app(test_apps):
-    """Test if `find_best_app` behaves as expected with different combinations of input."""
+    """Test if `find_best_app` behaves as expected with different combinations of input."""  # noqa: B950
     script_info = ScriptInfo()
 
     class Module:

tests/test_config.py

@@ -76,39 +76,32 @@ def test_config_from_class():
     common_object_test(app)
 
 
-def test_config_from_envvar():
-    env = os.environ
-    try:
-        os.environ = {}
-        app = flask.Flask(__name__)
-        with pytest.raises(RuntimeError) as e:
-            app.config.from_envvar("FOO_SETTINGS")
+def test_config_from_envvar(monkeypatch):
+    monkeypatch.setattr("os.environ", {})
+    app = flask.Flask(__name__)
+    with pytest.raises(RuntimeError) as e:
+        app.config.from_envvar("FOO_SETTINGS")
         assert "'FOO_SETTINGS' is not set" in str(e.value)
-        assert not app.config.from_envvar("FOO_SETTINGS", silent=True)
+    assert not app.config.from_envvar("FOO_SETTINGS", silent=True)
 
-        os.environ = {"FOO_SETTINGS": __file__.rsplit(".", 1)[0] + ".py"}
-        assert app.config.from_envvar("FOO_SETTINGS")
-        common_object_test(app)
-    finally:
-        os.environ = env
+    monkeypatch.setattr(
+        "os.environ", {"FOO_SETTINGS": __file__.rsplit(".", 1)[0] + ".py"}
+    )
+    assert app.config.from_envvar("FOO_SETTINGS")
+    common_object_test(app)
 
 
-def test_config_from_envvar_missing():
-    env = os.environ
-    try:
-        os.environ = {"FOO_SETTINGS": "missing.cfg"}
-        with pytest.raises(IOError) as e:
-            app = flask.Flask(__name__)
-            app.config.from_envvar("FOO_SETTINGS")
-        msg = str(e.value)
-        assert msg.startswith(
-            "[Errno 2] Unable to load configuration "
-            "file (No such file or directory):"
-        )
-        assert msg.endswith("missing.cfg'")
-        assert not app.config.from_envvar("FOO_SETTINGS", silent=True)
-    finally:
-        os.environ = env
+def test_config_from_envvar_missing(monkeypatch):
+    monkeypatch.setattr("os.environ", {"FOO_SETTINGS": "missing.cfg"})
+    with pytest.raises(IOError) as e:
+        app = flask.Flask(__name__)
+        app.config.from_envvar("FOO_SETTINGS")
+    msg = str(e.value)
+    assert msg.startswith(
+        "[Errno 2] Unable to load configuration " "file (No such file or directory):"
+    )
+    assert msg.endswith("missing.cfg'")
+    assert not app.config.from_envvar("FOO_SETTINGS", silent=True)
 
 
 def test_config_missing():

tests/test_helpers.py

@@ -182,7 +182,7 @@ class TestJSON(object):
 
     def test_jsonify_arrays(self, app, client):
         """Test jsonify of lists and args unpacking."""
-        l = [
+        a_list = [
             0,
             42,
             3.14,
@@ -196,16 +196,16 @@ class TestJSON(object):
 
         @app.route("/args_unpack")
         def return_args_unpack():
-            return flask.jsonify(*l)
+            return flask.jsonify(*a_list)
 
         @app.route("/array")
         def return_array():
-            return flask.jsonify(l)
+            return flask.jsonify(a_list)
 
         for url in "/args_unpack", "/array":
             rv = client.get(url)
             assert rv.mimetype == "application/json"
-            assert flask.json.loads(rv.data) == l
+            assert flask.json.loads(rv.data) == a_list
 
     def test_jsonify_date_types(self, app, client):
         """Test jsonify with datetime.date and datetime.datetime types."""
@@ -278,7 +278,7 @@ class TestJSON(object):
         assert rv == '<a ng-data=\'{"x": ["foo", "bar", "baz\\u0027"]}\'></a>'
 
     def test_json_customization(self, app, client):
-        class X(object):
+        class X(object):  # noqa: B903, for Python2 compatibility
             def __init__(self, val):
                 self.val = val
 
@@ -313,7 +313,7 @@ class TestJSON(object):
         assert rv.data == b'"<42>"'
 
     def test_blueprint_json_customization(self, app, client):
-        class X(object):
+        class X(object):  # noqa: B903, for Python2 compatibility
             def __init__(self, val):
                 self.val = val
 
@@ -352,6 +352,9 @@ class TestJSON(object):
         )
         assert rv.data == b'"<42>"'
 
+    @pytest.mark.skipif(
+        not has_encoding("euc-kr"), reason="The euc-kr encoding is required."
+    )
     def test_modified_url_encoding(self, app, client):
         class ModifiedRequest(flask.Request):
             url_charset = "euc-kr"
@@ -367,13 +370,10 @@ class TestJSON(object):
         assert rv.status_code == 200
         assert rv.data == u"정상처리".encode("utf-8")
 
-    if not has_encoding("euc-kr"):
-        test_modified_url_encoding = None
-
     def test_json_key_sorting(self, app, client):
         app.debug = True
 
-        assert app.config["JSON_SORT_KEYS"] == True
+        assert app.config["JSON_SORT_KEYS"]
         d = dict.fromkeys(range(20), "foo")
 
         @app.route("/")
@@ -858,7 +858,7 @@ class TestNoImports(object):
         try:
             flask.Flask("importerror")
         except NotImplementedError:
-            assert False, "Flask(import_name) is importing import_name."
+            AssertionError("Flask(import_name) is importing import_name.")
 
 
 class TestStreaming(object):

tests/test_instance_config.py

@@ -140,4 +140,4 @@ def test_meta_path_loader_without_is_package(request, modules_tmpdir):
     request.addfinalizer(sys.meta_path.pop)
 
     with pytest.raises(AttributeError):
-        import unimportable
+        import unimportable  # noqa: F401

tests/test_json_tag.py

@@ -48,7 +48,7 @@ def test_duplicate_tag():
 
 
 def test_custom_tag():
-    class Foo(object):
+    class Foo(object):  # noqa: B903, for Python2 compatibility
         def __init__(self, data):
             self.data = data
 

tests/test_regression.py

@@ -64,7 +64,7 @@ def test_memory_consumption():
     # This test only works on CPython 2.7.
     if sys.version_info >= (2, 7) and not hasattr(sys, "pypy_translation_info"):
         with assert_no_leak():
-            for x in range(10):
+            for _x in range(10):
                 fire()
 
 

tests/test_reqctx.py

@@ -191,7 +191,6 @@ class TestGreenletContextCopying(object):
         @app.route("/")
         def index():
             flask.session["fizz"] = "buzz"
-            reqctx = flask._request_ctx_stack.top.copy()
 
             @flask.copy_current_request_context
             def g():
@@ -228,7 +227,7 @@ def test_session_error_pops_context():
     @app.route("/")
     def index():
         # shouldn't get here
-        assert False
+        AssertionError()
 
     response = app.test_client().get("/")
     assert response.status_code == 500

tests/test_templating.py

@@ -398,12 +398,12 @@ def test_templates_auto_reload_debug_run(app, monkeypatch):
     monkeypatch.setattr(werkzeug.serving, "run_simple", run_simple_mock)
 
     app.run()
-    assert app.templates_auto_reload == False
-    assert app.jinja_env.auto_reload == False
+    assert not app.templates_auto_reload
+    assert not app.jinja_env.auto_reload
 
     app.run(debug=True)
-    assert app.templates_auto_reload == True
-    assert app.jinja_env.auto_reload == True
+    assert app.templates_auto_reload
+    assert app.jinja_env.auto_reload
 
 
 def test_template_loader_debugging(test_apps, monkeypatch):
@@ -412,7 +412,7 @@ def test_template_loader_debugging(test_apps, monkeypatch):
     called = []
 
     class _TestHandler(logging.Handler):
-        def handle(x, record):
+        def handle(self, record):
             called.append(True)
             text = str(record.msg)
             assert '1: trying loader of application "blueprintapp"' in text

tests/test_testing.py

@@ -211,13 +211,13 @@ def test_session_transactions_no_null_sessions():
 
     with app.test_client() as c:
         with pytest.raises(RuntimeError) as e:
-            with c.session_transaction() as sess:
+            with c.session_transaction():
                 pass
         assert "Session backend did not open a session" in str(e.value)
 
 
 def test_session_transactions_keep_context(app, client, req_ctx):
-    rv = client.get("/")
+    client.get("/")
     req = flask.request._get_current_object()
     assert req is not None
     with client.session_transaction():
@@ -227,7 +227,7 @@ def test_session_transactions_keep_context(app, client, req_ctx):
 def test_session_transaction_needs_cookies(app):
     c = app.test_client(use_cookies=False)
     with pytest.raises(RuntimeError) as e:
-        with c.session_transaction() as s:
+        with c.session_transaction():
             pass
     assert "cookies" in str(e.value)
 

tests/test_user_error_handler.py

@@ -149,7 +149,7 @@ def test_default_error_handler():
         return "bp-default"
 
     @bp.errorhandler(Forbidden)
-    def bp_exception_handler(e):
+    def bp_forbidden_handler(e):
         assert isinstance(e, Forbidden)
         return "bp-forbidden"
 
@@ -164,13 +164,13 @@ def test_default_error_handler():
     app = flask.Flask(__name__)
 
     @app.errorhandler(HTTPException)
-    def catchall_errorhandler(e):
+    def catchall_exception_handler(e):
         assert isinstance(e, HTTPException)
         assert isinstance(e, NotFound)
         return "default"
 
     @app.errorhandler(Forbidden)
-    def catchall_errorhandler(e):
+    def catchall_forbidden_handler(e):
         assert isinstance(e, Forbidden)
         return "forbidden"