Python code review checklist

1. Avoid bare except or broad except Exception that swallows errors
2. Pre-formatting log messages defeats deferred formatting
3. Avoid Any where a precise or object type fits
4. Return Self instead of the base class name
5. Avoid tempfile.mktemp (race-prone)
6. Do not pass untrusted input to getoutput or getstatusoutput
7. Do not store passwords in recoverable form
8. Passing non-literal strings to LiteralString parameters
9. Serializing NaN or Infinity produces invalid JSON
10. strict=False admits control characters
11. Unpickling untrusted data

11 rules to check in every Python pull request

Revix reviews Python pull requests against 386 rules. This page shows a short list we picked from them. Each rule has a bad example, a good example and a source. We check each rule against its source and update the list every month. Rules marked as flagged by Revix have come up in real reviews.

Avoid bare except or broad except Exception that swallows errors

Broad handlers mask programming errors such as NameError or TypeError and make failures surface far from their cause.

Avoid

try:
    value = int(raw)
except:
    value = 0

Prefer

try:
    value = int(raw)
except ValueError:
    value = 0

Flagged by Revix in real reviews · Anti-pattern · All current versions · Source: 8.5. Exception Chaining

Pre-formatting log messages defeats deferred formatting

Eager formatting wastes CPU in hot paths where debug logging is disabled in production.

Avoid

logger.debug(f'user {user!r} loaded')

Prefer

logger.debug('user %r loaded', user)

Flagged by Revix in real reviews · Performance · All current versions · Source: Logging HOWTO

Avoid Any where a precise or object type fits

Annotating with Any silently removes the protection the type checker provides at the call sites that receive the value.

Avoid

def load(x: Any) -> str: return x

Prefer

def load(x: object) -> str: return str(x)

Flagged by Revix in real reviews · Anti-pattern · All current versions · Source: typing — Support for type hints

Return Self instead of the base class name

Using the class name loses the subclass type for chained calls and alternative constructors.

Avoid

def set_name(self, n: str) -> 'Foo': self.n = n; return self

Prefer

def set_name(self, n: str) -> Self: self.n = n; return self

Flagged by Revix in real reviews · Best practice · All current versions · Source: typing — Support for type hints

Avoid tempfile.mktemp (race-prone)

The gap between naming and creating a file allows a symlink or pre-creation attack on the path.

Avoid

import tempfile
path = tempfile.mktemp()
open(path, 'w').write(data)

Prefer

import tempfile
f = tempfile.NamedTemporaryFile(delete=False)
f.write(data)
f.close()

Security · All current versions · Source: tempfile — SpooledTemporaryFile

Do not pass untrusted input to getoutput or getstatusoutput

These helpers give up the argument-list safety of subprocess and are unsafe with dynamic input.

Avoid

subprocess.getoutput('ls ' + path)

Prefer

subprocess.check_output(['ls', path], text=True)

Security · All current versions · Source: subprocess - Timeout Behavior

Do not store passwords in recoverable form

Recoverable password storage turns any database leak into an immediate account compromise.

Avoid

user.password = request.form['password']
user.save()

Prefer

import hashlib, os
salt = os.urandom(16)
user.password_hash = hashlib.pbkdf2_hmac('sha256', pw.encode(), salt, 600000)

Security · All current versions · Source: secrets — Generate secure random numbers for managing secrets

Passing non-literal strings to LiteralString parameters

Sensitive APIs typed with LiteralString rely on that guarantee to prevent injection.

Avoid

run_query(f'SELECT * FROM t WHERE n = {name}')

Prefer

cursor.execute('SELECT * FROM t WHERE n = ?', (name,))

Security · All current versions · Source: typing — Support for type hints

Serializing NaN or Infinity produces invalid JSON

The default silently produces output that is not valid JSON, so the error appears far from the source, often in a downstream consumer.

Avoid

payload = json.dumps({'avg': float('nan')})

Prefer

payload = json.dumps({'avg': avg}, allow_nan=False)

Security · All current versions · Source: json — JSON encoder and decoder

strict=False admits control characters

Relaxed parsing of untrusted input weakens the validity guarantee the decoder otherwise gives.

Avoid

json.JSONDecoder(strict=False).decode(body)

Prefer

json.JSONDecoder().decode(body)

Security · All current versions · Source: json — JSON encoder and decoder

Unpickling untrusted data

Unpickling attacker-controlled bytes gives the attacker code execution in your process.

Avoid

obj = pickle.loads(request.body)

Prefer

obj = json.loads(request.body)  # or verify an hmac signature before pickle.loads

Security · All current versions · Source: pickle - Python object serialization

How Revix applies these rules in pull requests

Revix matches these rules against the code each pull request changes. When a comment rests on one, it names the rule, so the author can read the reasoning and the source instead of taking the bot's word for it.

+try:

+ value = int(raw)

+except:

+ value = 0

revixappdevBot

issue: Broad handlers mask programming errors such as NameError or TypeError and make failures surface far from their cause.

📚 Rule (Python): Avoid bare except or broad except Exception that swallows errors

Reviews run natively on GitHub, GitLab and Bitbucket.

Related checklists

• JavaScript code review checklist
• Next.js code review checklist
• React code review checklist
• TypeScript code review checklist