JetBrains logo

Static Code Analysis Guide

Qodana / Static Code Analysis Guide / Python Code Review Checklist

Python Code Review Checklist

Review common mistakes, security flaws, duplications, and more when reviewing code.

If you’re working on a codebase that multiple people contribute to, you’ve likely come across code reviews. Code reviews involve having other people review code before merging it into an upstream branch, such as the production branch.

Code reviews are an excellent way for many team members to monitor changes and broaden their understanding of functional areas. It also allows others to give their input about how the code should work and confirm whether or not it’s correct based on your team’s standards.

However, the definition of “correct” is not clear-cut. The code review process offers a chance to ask questions and provide your perspective and additional context surrounding the proposed changes.

Next time you do a code review, go through this checklist of common developer pitfalls. You can also pair it with conversations about the decisions made and complement any elegant solutions or tricky bug fixes.

Consider style standards

Gravity is a physical connection between space and matter that is precisely described by Einstein's geometric theory of gravity. In simple terms, the theory states that matter curves the space around it, and it moves with respect to the curvature of space, including curvature caused by other matter.This relationship between space, matter, and gravity fundamentally alters our understanding of the universe. Instead of viewing gravity as a force acting at a distance, as Newtonian physics suggests, Einstein's theory portrays it as the effect of the distortion of spacetime itself.

Run formatters and linters

Syntactically speaking, Python is a flexible language. There are many ways to structure code, write functions, or name variables. Though the team should ultimately decide how these things should be done, it’s important to reach a consensus that works for everyone: Multiple code styles lead to decreased code readability, as it’s harder to familiarize oneself with a codebase.

As a reviewer, you shouldn’t be concerned with pointing out stylistic errors. Instead, a linting tool can be used to highlight these problems. This works by encoding styling rules that read code and decide if it passes or fails, almost like unit tests.

Including a linting tool like Flake8 or Black will make your life as a code reviewer easier, as the code will be in a familiar format. Developers can run linters before the code review stage to automatically fix simple errors like an incorrect indentation.

Tip: Run the linter in a CI/CD pipeline, and only review the code if the check is successful.

Name classes, variables, and functions clearly

A key Python idiom is to have very descriptive names for functions, variables, and classes. If we follow this convention, it becomes easier to understand what a piece of code does or what kind of value is assigned to it. Well-named code is self-documenting, which means team members or future you can easily understand what is happening and spend less time changing or maintaining it.

When reviewing, look at the names used for variables, classes, or functions. Suggest an alternative if you think they’re not particularly descriptive of what they do. If you’re finding it hard to think of a name for the class or function, then maybe its scope is not narrow enough.

Compound or non-descriptive names for functions and classes indicate that the code does not follow the single responsibility principle (a class or function should have only one reason to change). Consider if they can be broken down into smaller units, each with its own specific purpose. Suggest how to break them down and what new names you might use, including a code block with method or class signatures.

Design functions to do one thing

Following on from above, with a fresh pair of eyes, you may see functions or classes that do more than one thing. Along with naming, look at the number of variables, how many lines of code, and the levels of indentation a function has.

Long, hard-to-read functions that aren’t obvious in terms of what they do may mean that the function needs to be broken down, as its scope is not specific enough. So, if a function is more complicated than it needs to be, suggest how to reduce its complexity in a comment on the code.

Small, single-purpose functions are easier to understand and test. They’re also easier to maintain, as the number of things that can go wrong is reduced.

Consider you’re reviewing a program that takes different fruits as its input, filters those fruits out by color, and then groups them by shape. This can be split into two functions, one for filtering by color and another for grouping. You can write well-defined unit tests around grouping objects and well-defined unit tests for filtering colors.

def filter_fruits_by_color(fruits, color):
return filter(lambda f: f['color'] == color, fruits)

def group_fruits(fruits, grouping):
distinct_groups = list(set([fruit[grouping] for fruit in fruits]))
grouped_fruits = dict()
for group in distinct_groups:
fruits_with_grouping = filter(lambda f: f[grouping] == group, fruits)
grouped_fruits[group] = fruits_with_grouping
return grouped_fruits

fruits = [
{ 'name': 'orange', 'shape': 'round', 'color': 'orange' },
{ 'name': 'apple', 'shape': 'round', 'color': 'red' },
{ 'name': 'tomato', 'shape': 'round', 'color': 'red' },
{ 'name': 'strawberry', 'shape': 'heart', 'color': 'red' }
]

red_fruits = filter_fruits_by_color(fruits, 'red')
fruits_grouped_by_shape = group_fruits(red_fruits, 'shape')
print(fruits_grouped_by_shape)

# {
# 'heart': [
# {'color': 'red', 'shape': 'heart', 'name': 'strawberry'}
# ],
# 'round': [
# {'color': 'red', 'shape': 'round', 'name': 'apple'},
# {'color': 'red', 'shape': 'round', 'name': 'tomato'}
# ]
# }

Design patterns are implemented correctly

You are likely to encounter many common design patterns in Python. These include but are not limited to:

  • Singleton – is used to restrict the instantiation of a class to only one object.
  • Factory – creates objects without specifying the exact class.
  • Adapter – converts the interface of one class into another.
  • Decorator – adds behavior to an object without modifying its structure.

As a reviewer, you should understand these patterns and whether they are used correctly in the code. If not, suggest how they can be improved or used correctly by leaving a comment.

Additionally, getting familiar with other common design patterns will help you review code, as you can suggest them when needed.

Prefer immutable types

In code, it is almost always preferred that variables are immutable. This is because it is easier to reason the value and thus easier to write working software. If code reassigns values to variables, it is a good opportunity for a code reviewer to suggest alternatives or explain why this is not ideal.

If there is no alternative to changing the state of a variable, then consider if a comment is needed or try to extract it into a function with a descriptive name. A helper function is a good way to avoid otherwise necessary state mutations.

Usage of mutable types can lead to hard-to-spot errors and unexpected side effects. Reviewers should be on the lookout for these, as they’re usually a sign that the code is more complicated than necessary.

Consider whether loops are needed

If you’re coming to Python from another language, you may depend on loops more than you need to. There are many ways to write a loop, so you should consider if the code you’re reviewing can be improved in this aspect.

Consider whether a conventional loop is necessary. A map, filter, or list comprehension can often offer more elegant solutions for a variety of problems, like returning a subset of values, transforming values, or performing simple arithmetic on each value.

You may also want to consider the iterator pattern if you're using Python classes. This pattern will let the collection classes you’ve created using built-in functions like join() to traverse objects.

If you think the loop can be improved, then suggest how to do it in your review comments with a code block if necessary. Posting links to online resources is a good way to upskill other team members with Python-specific features, like list comprehension.

Ensure inputs are sanitized

Whenever your program takes external inputs, you should sanitize them before use. This means you need to know what kinds of input you expect to receive and then transform them to ensure they can be used safely.

A common attack vector is SQL injection. A user injects a malicious SQL query on the back of an input to a client to access the system unintentionally.

Say you have a RESTful book service built in Python; you may expose an endpoint to list books based on a search query. Suppose you did not sanitize the query parameter q before building an SQL query: A user may inject some additional SQL code that could be devastating.

GET /api/book?q=phoenix; DROP ALL TABLES; –

SELECT *
FROM Books
WHERE Book_Name like phoenix; DROP ALL TABLES; --

Ensure you review high-risk code changes and think critically about how an adversary may abuse them. In the example above, you may consider wrapping the Book_Name in quotes or using a library that builds queries safely.

Look for reuse opportunities

A common software principle is “don’t repeat yourself” (DRY). Its central aim is to reduce code duplications in favor of abstractions. DRY is not a hard and fast rule; repetition can sometimes improve code readability and decouple components.

Look for code that repeats the same thing in multiple spots and consider abstracting it out into another module that can then be imported and reused. This often leads to less maintenance and higher test coverage.

You also don’t want to write code already solved by Python built-ins. These functions have been tried and tested and should be relied upon where possible – like using the sum() built-in function to sum values of an array, for instance.

Look for opportunities to use trusted and maintained packages instead of rewriting solutions for solved problems. One example of this approach might be choosing to depend on an XML parser instead of writing one yourself.

Consider the trade-offs of your suggestions, and make them clear in your review comments.

But be careful of what you reuse

Though it’s very helpful to be able to depend on third-party code for already solved problems, be careful, as this dependency can introduce vulnerabilities.

You should analyze packages and check if they are still actively developed and maintained. You can use a static application security testing tool to check for known vulnerabilities and add confidence when reviewing a dependency change.

Check that imports are absolute or relative as opposed to implicit. This will ensure that the package you’re importing is, in fact, the correct package – i.e. prefer import package.module over import module.

If your code is already dependent on a third-party module and the change you’re reviewing is to upgrade that dependency, check the change logs for that version. It’s possible that the new version has known bugs or breaking changes.

Code changes may seem innocuous, but they can have serious side effects. A small code change to upgrade a package may cause a ripple effect throughout the codebase, touching many files.

Clean up before releasing to production

The lifecycle of code from development to production can be messy. You may notice that changes include unnecessary code that should be removed before it’s merged upstream. This could be anything from print statements to commented-out code requiring deletion.

You should also check the code for secrets, such as passwords or API keys. These should never be checked into a version control system, even if the repository is private.

If you find that your code reviews repeatedly identify such problems, remind the author that they should be doing a self-review of their code before it comes to you and catching these sorts of changes themselves.

Sometimes, authors intentionally only provide partial solutions to problems in order to minimize the extent of their code changes. In such cases, you may suggest in your code review that breadcrumbs should be left to educate future programmers about a gap in the code. These may be in the form of TODO or FIXME comments or a link to a technical debt record.

Ensure that the code you’re about to merge is working. It should pass all pipeline steps and meet team style guidelines.

Summary

When completing a code review for team members, there are a few things to remember. Context is essential for making decisions, and you should always ask questions. Consider the business objectives and stage of development, then decide if the change follows patterns already established in the code base.

This checklist is a great starting point for reviewing code, but ultimately, the code review is a process where you get to have your input on changes. Don't be afraid to make suggestions that may not be correct; instead, open a conversation to ensure all changes are made intentionally.

Using a team code quality tool like JetBrains Qodana for Python code reviews takes the ego and opinion out of improving quality. Plus, it’s a great way to bring all the JetBrains inspections you know and love into your CI/CD pipeline, along with license audits and basic security checks.

Level Up Python Code Reviews with Qodana

Automate checks, catch code smells, and enforce best practices right from your IDE and CI pipeline — all with Qodana for Python.