Enable many more Ruff linter rules

Author: seanh

Diff for: .cookiecutter/includes/ruff/lint/per_file_ignores/tests/tail

@@ -0,0 +1,6 @@
+"PT006", # Enforces a consistent style for the type of the `argnames` parameter to
+         # pytest.mark.parametrize. We have too many pre-existing violations of
+         # this.
+"PT007", # Enforces a consistent style for the type of the `argvalues` parameter to
+         # pytest.mark.parametrize. We have too many pre-existing violations of
+         # this.

Diff for: bin/run_data_task.py

@@ -4,7 +4,7 @@
 This is a general mechanism for running tasks defined in SQL, however it's
 currently only used to perform the aggregations and mappings required for
 reporting.
-"""
+"""  # noqa: EXE002
 
 from argparse import ArgumentParser
 
@@ -60,18 +60,18 @@ def main():
         )
 
         # Run the update in a transaction, so we roll back if it goes wrong
-        with request.db.bind.connect() as connection:
+        with request.db.bind.connect() as connection:  # noqa: SIM117
             with connection.begin():
                 for script in scripts:
                     if args.no_python and isinstance(script, PythonScript):
-                        print(f"Skipping: {script}")
+                        print(f"Skipping: {script}")  # noqa: T201
                         continue
 
                     for step in script.execute(connection, dry_run=args.dry_run):
                         if args.dry_run:
-                            print("Dry run!")
+                            print("Dry run!")  # noqa: T201
 
-                        print(step.dump(indent="    ") + "\n")
+                        print(step.dump(indent="    ") + "\n")  # noqa: T201
 
 
 if __name__ == "__main__":

Diff for: h/_version.py

@@ -15,23 +15,24 @@
 
 
 def fetch_git_ref():
-    return subprocess.check_output(
-        ["git", "rev-parse", "--short", "HEAD"], stderr=DEVNULL
+    return subprocess.check_output(  # noqa: S603
+        ["git", "rev-parse", "--short", "HEAD"],  # noqa: S607
+        stderr=DEVNULL,
     ).strip()
 
 
 def fetch_git_date(ref):
-    output = subprocess.check_output(["git", "show", "-s", "--format=%ct", ref])
-    return datetime.datetime.fromtimestamp(int(output))
+    output = subprocess.check_output(["git", "show", "-s", "--format=%ct", ref])  # noqa: S603, S607
+    return datetime.datetime.fromtimestamp(int(output))  # noqa: DTZ006
 
 
 def fetch_git_dirty():
     # Ensure git index is up-to-date first. This usually isn't necessary, but
     # can be needed inside a docker container where the index is out of date.
-    subprocess.call(["git", "update-index", "-q", "--refresh"])
-    dirty_tree = bool(subprocess.call(["git", "diff-files", "--quiet"]))
+    subprocess.call(["git", "update-index", "-q", "--refresh"])  # noqa: S603, S607
+    dirty_tree = bool(subprocess.call(["git", "diff-files", "--quiet"]))  # noqa: S603, S607
     dirty_index = bool(
-        subprocess.call(["git", "diff-index", "--quiet", "--cached", "HEAD"])
+        subprocess.call(["git", "diff-index", "--quiet", "--cached", "HEAD"])  # noqa: S603, S607
     )
     return dirty_tree or dirty_index
 
@@ -45,11 +46,11 @@ def git_version():
 
 def git_archive_version():  # pragma: no cover
     ref = VERSION_GIT_REF
-    date = datetime.datetime.fromtimestamp(int(VERSION_GIT_DATE))
+    date = datetime.datetime.fromtimestamp(int(VERSION_GIT_DATE))  # noqa: DTZ006
     return pep440_version(date, ref)
 
 
-def pep440_version(date, ref, dirty=False):
+def pep440_version(date, ref, dirty=False):  # noqa: FBT002
     """Build a PEP440-compliant version number from the passed information."""
     return f"{date.strftime('%Y%m%d')}+g{ref}{'.dirty' if dirty else ''}"
 

Diff for: h/accounts/schemas.py

@@ -1,7 +1,7 @@
 import codecs
 import logging
 from datetime import datetime, timedelta
-from functools import lru_cache
+from functools import cache
 
 import colander
 import deform
@@ -24,18 +24,18 @@
 log = logging.getLogger(__name__)
 
 
-@lru_cache(maxsize=None)
+@cache
 def get_blacklist():
     # Try to load the blacklist file from disk. If, for whatever reason, we
     # can't load the file, then don't crash out, just log a warning about
     # the problem.
     try:
         with codecs.open("h/accounts/blacklist", encoding="utf-8") as handle:
             blacklist = handle.readlines()
-    except (IOError, ValueError):  # pragma: no cover
+    except (IOError, ValueError):  # pragma: no cover  # noqa: UP024
         log.exception("unable to load blacklist")
         blacklist = []
-    return set(line.strip().lower() for line in blacklist)
+    return set(line.strip().lower() for line in blacklist)  # noqa: C401
 
 
 def unique_email(node, value):
@@ -68,7 +68,7 @@ def unique_username(node, value):
         )
         # 31 days is an arbitrary time delta that should be more than enough
         # time for all the previous user's data to be expunged.
-        .where(models.UserDeletion.requested_at > datetime.now() - timedelta(days=31))
+        .where(models.UserDeletion.requested_at > datetime.now() - timedelta(days=31))  # noqa: DTZ005
     ).first():
         raise exc
 

Diff for: h/accounts/util.py

@@ -23,10 +23,10 @@ def validate_url(url):
         parsed_url = urlparse("http://" + url)
 
     if not re.match("https?", parsed_url.scheme):
-        raise ValueError('Links must have an "http" or "https" prefix')
+        raise ValueError('Links must have an "http" or "https" prefix')  # noqa: EM101, TRY003
 
     if not parsed_url.netloc:
-        raise ValueError("Links must include a domain name")
+        raise ValueError("Links must include a domain name")  # noqa: EM101, TRY003
 
     return parsed_url.geturl()
 
@@ -44,10 +44,10 @@ def validate_orcid(orcid):
     orcid_regex = r"\A[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{3}[0-9X]\Z"
 
     if not re.match(orcid_regex, orcid):
-        raise ValueError(f"The format of this ORCID is incorrect: {orcid}")
+        raise ValueError(f"The format of this ORCID is incorrect: {orcid}")  # noqa: EM102, TRY003
 
     if _orcid_checksum_digit(orcid[:-1]) != orcid[-1:]:
-        raise ValueError(f"{orcid} is not a valid ORCID")
+        raise ValueError(f"{orcid} is not a valid ORCID")  # noqa: EM102, TRY003
 
     return True
 

Diff for: h/activity/bucketing.py

@@ -146,7 +146,7 @@ def next(self, annotation):
             if timeframe.within_cutoff(annotation):
                 return timeframe
 
-        cutoff_time = datetime.datetime(
+        cutoff_time = datetime.datetime(  # noqa: DTZ001
             year=annotation.updated.year, month=annotation.updated.month, day=1
         )
         timeframe = Timeframe(annotation.updated.strftime("%b %Y"), cutoff_time)
@@ -179,4 +179,4 @@ def bucket(annotations):
 
 
 def utcnow():  # pragma: no cover
-    return datetime.datetime.utcnow()
+    return datetime.datetime.utcnow()  # noqa: DTZ003

Diff for: h/activity/query.py

@@ -17,7 +17,7 @@
 from h.services.annotation_read import AnnotationReadService
 
 
-class ActivityResults(
+class ActivityResults(  # noqa: SLOT002
     namedtuple("ActivityResults", ["total", "aggregations", "timeframes"])  # noqa: PYI024
 ):
     pass

Diff for: h/app.py

@@ -63,7 +63,7 @@ def includeme(config):  # pragma: no cover
 
     config.add_settings(
         {
-            "tm.manager_hook": lambda request: transaction.TransactionManager(),
+            "tm.manager_hook": lambda request: transaction.TransactionManager(),  # noqa: ARG005
             "tm.annotate_user": False,
         }
     )

Diff for: h/celery.py

@@ -179,7 +179,7 @@ def add_task_name_and_id_to_log_messages(
 
     root_loggers_handler.setFormatter(
         logging.Formatter(
-            "[%(asctime)s: %(levelname)s/%(processName)s] "
+            "[%(asctime)s: %(levelname)s/%(processName)s] "  # noqa: ISC003
             + f"{task.name}[{task_id}] "
             + "%(message)s"
         )

Diff for: h/cli/__init__.py

@@ -22,7 +22,7 @@
 )
 
 
-def bootstrap(app_url, dev=False):
+def bootstrap(app_url, dev=False):  # noqa: FBT002
     """
     Bootstrap the application from the given arguments.
 
@@ -34,7 +34,7 @@ def bootstrap(app_url, dev=False):
         if dev:
             app_url = "http://localhost:5000"
         else:
-            raise click.ClickException("the app URL must be set in production mode!")
+            raise click.ClickException("the app URL must be set in production mode!")  # noqa: EM101, TRY003
 
     config = "conf/development.ini" if dev else "conf/production.ini"
 

Diff for: h/cli/commands/authclient.py

@@ -36,7 +36,7 @@ def authclient():
     help="An allowable grant type",
 )
 @click.pass_context
-def add(ctx, name, authority, type_, redirect_uri, grant_type):
+def add(ctx, name, authority, type_, redirect_uri, grant_type):  # noqa: PLR0913
     """Create a new OAuth client."""
     request = ctx.obj["bootstrap"]()
 

Diff for: h/cli/commands/create_annotations.py

@@ -15,13 +15,13 @@ def create_annotations(ctx, number):
     tm = request.tm
 
     for _ in range(number):
-        created = updated = datetime.datetime(
-            year=random.randint(2015, 2020),
-            month=random.randint(1, 12),
-            day=random.randint(1, 27),
-            hour=random.randint(1, 12),
-            minute=random.randint(0, 59),
-            second=random.randint(0, 59),
+        created = updated = datetime.datetime(  # noqa: DTZ001
+            year=random.randint(2015, 2020),  # noqa: S311
+            month=random.randint(1, 12),  # noqa: S311
+            day=random.randint(1, 27),  # noqa: S311
+            hour=random.randint(1, 12),  # noqa: S311
+            minute=random.randint(0, 59),  # noqa: S311
+            second=random.randint(0, 59),  # noqa: S311
         )
         db.add(
             factories.Annotation.build(created=created, updated=updated, shared=True)

Diff for: h/cli/commands/devdata.py

@@ -55,7 +55,7 @@ def create_all(self):
             elif type_ == "restricted_group":
                 self.upsert_restricted_group(data_dict)
             else:
-                raise RuntimeError(f"Unrecognized type: {type_}")
+                raise RuntimeError(f"Unrecognized type: {type_}")  # noqa: EM102, TRY003
 
         self.tm.commit()
 
@@ -113,7 +113,7 @@ def upsert_group(self, group_data, group_create_method):
         creator = models.User.get_by_username(
             self.db, group_data.pop("creator_username"), group_data["authority"]
         )
-        assert creator
+        assert creator  # noqa: S101
 
         organization = (
             self.db.query(models.Organization)
@@ -145,23 +145,24 @@ def setattrs(object_, attrs):
 def devdata(ctx):
     with tempfile.TemporaryDirectory() as tmpdirname:
         # The directory that we'll clone the devdata git repo into.
-        git_dir = os.path.join(tmpdirname, "devdata")
+        git_dir = os.path.join(tmpdirname, "devdata")  # noqa: PTH118
 
         # Clone the private devdata repo from GitHub.
         # This will fail if Git->GitHub HTTPS authentication isn't set up or if
         # your GitHub account doesn't have access to the private repo.
-        subprocess.check_call(
-            ["git", "clone", "https://github.com/hypothesis/devdata.git", git_dir]
+        subprocess.check_call(  # noqa: S603
+            ["git", "clone", "https://github.com/hypothesis/devdata.git", git_dir]  # noqa: S607
         )
 
         # Copy environment variables file into place.
         shutil.copyfile(
-            os.path.join(git_dir, "h", "devdata.env"),
-            os.path.join(pathlib.Path(h.__file__).parent.parent, ".devdata.env"),
+            os.path.join(git_dir, "h", "devdata.env"),  # noqa: PTH118
+            os.path.join(pathlib.Path(h.__file__).parent.parent, ".devdata.env"),  # noqa: PTH118
         )
 
-        with open(
-            os.path.join(git_dir, "h", "devdata.json"), "r", encoding="utf8"
+        with open(  # noqa: PTH123
+            os.path.join(git_dir, "h", "devdata.json"),  # noqa: PTH118
+            encoding="utf8",
         ) as handle:
             DevDataFactory(
                 ctx.obj["bootstrap"](),

Diff for: h/cli/commands/move_uri.py

@@ -40,7 +40,7 @@ def move_uri(ctx, old, new):
     )
     answer = click.prompt(prompt, default="n", show_default=False)
     if answer != "y":
-        print("Aborted")
+        print("Aborted")  # noqa: T201
         return
 
     for annotation in annotations:

Diff for: h/cli/commands/normalize_uris.py

@@ -8,7 +8,7 @@
 from h.util import uri
 
 
-class Window(namedtuple("Window", ["start", "end"])):  # noqa: PYI024
+class Window(namedtuple("Window", ["start", "end"])):  # noqa: PYI024, SLOT002
     pass
 
 

Diff for: h/cli/commands/search.py

@@ -23,4 +23,4 @@ def update_settings(ctx):
     try:
         config.update_index_settings(request.es)
     except RuntimeError as exc:
-        raise click.ClickException(str(exc))
+        raise click.ClickException(str(exc))  # noqa: B904

Diff for: h/cli/commands/user.py

@@ -39,7 +39,7 @@ def add(ctx, username, email, password, authority):
         message = (
             f"could not create user due to integrity constraint.\n\n{upstream_error}"
         )
-        raise click.ClickException(message)
+        raise click.ClickException(message)  # noqa: B904
 
     click.echo(f"{username} created", err=True)
 
@@ -92,8 +92,8 @@ def password(ctx, username, authority, password):
 
     user = models.User.get_by_username(request.db, username, authority)
     if user is None:
-        raise click.ClickException(
-            f'no user with username "{username}" and authority "{authority}"'
+        raise click.ClickException(  # noqa: TRY003
+            f'no user with username "{username}" and authority "{authority}"'  # noqa: EM102
         )
 
     password_service.update_password(user, password)

Diff for: h/config.py

@@ -23,7 +23,7 @@
 )
 
 
-def configure(environ=None, settings=None):
+def configure(environ=None, settings=None):  # noqa: PLR0915
     if environ is None:  # pragma: no cover
         environ = os.environ
     if settings is None:  # pragma: no cover

Diff for: h/db/__init__.py

@@ -20,7 +20,7 @@
 from sqlalchemy import text
 from sqlalchemy.orm import declarative_base, sessionmaker
 
-__all__ = ("Base", "Session", "pre_create", "post_create", "create_engine")
+__all__ = ("Base", "Session", "create_engine", "post_create", "pre_create")
 
 log = logging.getLogger(__name__)