Fix mistakes hidden in your Django codebase
Trusted by over 3000 teams to improve their Django code.
Securely check your codebase for free instantly.
We're a proud corporate member of the Django Software Foundation.
We've improved thousands of codebases including
Chrome
PyTorch
Sentry
Tensorflow
Firefox
It caught malformed test assertions that were incorrect. The automated codebase audit tool is so handy!
Jared Lockhart, Senior Software Engineer at MozillaExtremely positive. Suggested useful changes, giving our senior developers time back.
Jon Atkinson, Technical Director at GiantUncovered some bugs in the tests - thanks a lot!
Patrick von Platen, Research Engineer at Hugging FaceWorks with tools you use
- Detect typos
- Avoid mistakes
- Prevent bugs
- Improve tests
- Read more
- Review PRs
- One click fix
- Clear advice
- Delta aware
- Read more
- Review PRs
- Fix suggested
- Clear advice
- Delta aware
- Read more
A quick example
This is a sample of the kind of advice you can expect from Code Review Doctor.
41 | 41 | ⠀ | |
42 | 42 | # https://docs.djangoproject.com/en/dev/ref/settings/#append-slash | |
43 | 43 | APPEND_SLASH = True |
Stating defaults add complexity when reading the code but does not change Django's behaviour.
Read more- | APPEND_SLASH = True |
44 | 44 | ⠀ | |
45 | 45 | # Application definition |
62 | 62 | 'whitenoise.middleware.WhiteNoiseMiddleware', |
The order of middleware affections the outcome. Some middleware are dependant on the functionality of other middleware. For example a middleware that requires usage of request.session should come after the SessionMiddleware.
Read moreYour website is vulnerable to a number of common hacker attacks because MIDDLEWARE setting is missing django.middleware.security.SecurityMiddleware.
+ | 'django.middleware.security.SecurityMiddleware', | |||
Expand 12 lines ... |
63 | 63 | 'django.contrib.sessions.middleware.SessionMiddleware', | |
64 | 64 | 'django.middleware.cache.UpdateCacheMiddleware', | |
65 | 65 | 'django.middleware.common.CommonMiddleware', | |
66 | 66 | 'django.middleware.cache.FetchFromCacheMiddleware', | |
67 | 67 | 'django.middleware.csrf.CsrfViewMiddleware', | |
68 | 68 | 'django.contrib.auth.middleware.AuthenticationMiddleware', | |
69 | 69 | 'django.contrib.messages.middleware.MessageMiddleware', | |
70 | 70 | 'django.middleware.clickjacking.XFrameOptionsMiddleware', | |
71 | 71 | ] | |
72 | 72 | ||
73 | 73 | ROOT_URLCONF = 'config.urls' | |
74 | 74 | ||
75 | 75 | TEMPLATES = [ |
DIRS must be absolute paths. Relative paths will not work.
Read more76 | 76 | { | |
77 | 77 | 'BACKEND': 'django.template.backends.django.DjangoTemplates', | |
78 | 78 | 'DIRS': ['core/templates'], | |
79 | 79 | 'APP_DIRS': True, | |
80 | 80 | 'OPTIONS': { | |
81 | 81 | 'context_processors': [ | |
82 | 82 | 'django.template.context_processors.debug', | |
83 | 83 | 'django.template.context_processors.request', | |
84 | 84 | 'django.template.context_processors.i18n', | |
85 | 85 | 'django.contrib.auth.context_processors.auth', | |
86 | 86 | 'django.contrib.messages.context_processors.messages', | |
87 | 87 | 'staticpages.context_processors.get_pages', | |
88 | 88 | 'staticpages.context_processors.get_categories', | |
89 | 89 | 'staticpages.context_processors.get_urls', | |
90 | 90 | ], | |
91 | 91 | }, | |
92 | 92 | }, | |
93 | 93 | ] | |
94 | 94 | ||
95 | 95 | # Static files served with Whitenoise and AWS Cloudfront |
119 | 119 | SECURE_CONTENT_TYPE_NOSNIFF = True | |
120 | 120 | ||
121 | 121 | USE_I18N = True |
Stating defaults add complexity when reading the code but does not change Django's behaviour.
Read more- | USE_I18N = True |
122 | 122 | ⠀ | |
123 | 123 | DIRECTORY_CONSTANTS_URL_GREAT_DOMESTIC = env.str( |
148 | 148 | SSO_PROFILE_URL = 'https://profile.com' |
Your website is vulnerable because the SESSION_COOKIE_SECURE setting is not set - so hackers have an easier time stealing your users' session cookies on HTTP connections.
Your website is vulnerable because the CSRF_COOKIE_SECURE setting is not set - so hackers have an easier time stealing your CSRF cookies on HTTP connections, allowing them to circumvent your CSRF protection.
+ | |
+ | |
+ | SESSION_COOKIE_SECURE = os.getenv('SESSION_COOKIE_SECURE_ENABLED') != 'False' |
+ | |
+ | CSRF_COOKIE_SECURE = os.getenv('CSRF_COOKIE_SECURE_ENABLED') != 'False' |
16 | 16 | user = PlatformUser.objects.get(pk=1) | |
17 | 17 | ||
18 | 18 | if user.group.id != get_request_user_organisation_id(request): |
When working with foreign keys, accessing the related field will result in a database read. That can be eliminated by using *_id, which is the foreign key value that Django has already cached on the object to make this scenario more efficient.
- | if user.group.id != get_request_user_organisation_id(request): | |||
+ | if user.group_id != get_request_user_organisation_id(request): | |||
Expand 3 lines ... |
19 | 19 | raise PermissionDenied() | |
20 | 20 | ||
21 | 21 | return Response() |
25 | 25 | ⠀ | |
26 | 26 | def post(self, request, pk): | |
27 | 27 | if PlatformUser.objects.filter(group=1).count() > 0: |
Comparing queryset.count() is less efficient than checking queryset.exists(), so use querySet.count() if you only want the count, and use queryset.exists() if you only want to find out if at least one result exists.
- | if PlatformUser.objects.filter(group=1).count() > 0: | |||
+ | if PlatformUser.objects.filter(group=1).exists(): | |||
Expand 2 lines ... |
28 | 28 | raise PermissionDenied() | |
29 | 29 | return Response() | |
30 | 30 | ⠀ |
34 | 34 | ⠀ | |
35 | 35 | def post(self, request, pk): | |
36 | 36 | if PlatformUser.objects.filter(group=1): |
This can load every row in the database table into memory because the queryset is evaluated. Checking if a queryset is truthy/falsey is much less efficient than checking queryset.exists().
- | if PlatformUser.objects.filter(group=1): | |||
+ | if PlatformUser.objects.filter(group=1).exists(): | |||
Expand 2 lines ... |
37 | 37 | raise PermissionDenied() | |
38 | 38 | return Response() | |
39 | 39 | ⠀ |
45 | 45 | user = PlatformUser.objects.all().order_by('?')[0] |
Using order_by('?') can be very inefficient if you have lots of rows in the table. Moving the randomness to the application layer will probably give significant a performance improvement.
- | user = PlatformUser.objects.all().order_by('?')[0] |
+ | user = PlatformUser.objects.all()[random.randint(0, PlatformUser.objects.count() - 1)] |
31 | 31 | path("submit_job_raw/", views.View3.as_view(), name="submit-job"), |
URL names must be unique otherwise reverse('url_name') and {% url 'url_name' %} will link to the "wrong" page half of the time.
17 | 17 | def view_context(self, context): | |
18 | 18 | context['title_complete'] = 'LOCAIS DE ESTOQUE' | |
19 | 19 | context['add_url'] = reverse_lazy('addlocalview') |
Using reverse(...) is better than using reverse_lazy(...) when the url is being reversed after URLConf has been loaded.
- | context['add_url'] = reverse_lazy('addlocalview') |
+ | context['add_url'] = reverse('addlocalview') |
20 | 20 | return context | |
21 | 21 | ⠀ |
25 | 25 | items = Group.objects.filter(oferta_trimestral=oferta,) | |
26 | 26 | ||
27 | 27 | if len(items) == 0: |
len(queryset) performs the count at application level. That is much less efficient than doing queryset.count(), which does the calculation at database level and just returns the count.
Comparing queryset.count() is less efficient than checking queryset.exists(), so use querySet.count() if you only want the count, and use queryset.exists() if you only want to find out if at least one result exists.
- | if len(items) == 0: | |||
+ | if not items.exists(): | |||
Expand 3 lines ... |