Skip to content

Commit

Permalink
Add new extension for detecting integer comparisons against zero
Browse files Browse the repository at this point in the history
  • Loading branch information
atodorov committed Dec 30, 2016
1 parent 2304d03 commit 6a8a69d
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 1 deletion.
3 changes: 2 additions & 1 deletion CONTRIBUTORS.txt
Expand Up @@ -123,7 +123,8 @@ Order doesn't matter (not that much, at least ;)

* Alexander Todorov: added new error conditions to 'bad-super-call',
Added new check for incorrect len(SEQUENCE) usage,
Added new extension for comparison against empty string constants
Added new extension for comparison against empty string constants,
Added new extension which detects comparing integers to zero

* Erik Eriksson - Added overlapping-except error check.

Expand Down
2 changes: 2 additions & 0 deletions ChangeLog
Expand Up @@ -29,6 +29,8 @@ Release date: tba

* Added new extension to detect comparisons against empty string constants

* Added new extension to detect comparisons of integers against zero

* Added new error conditions for 'bad-super-call'

Now detects ``super(type(self), self)`` and ``super(self.__class__, self)``
Expand Down
34 changes: 34 additions & 0 deletions doc/whatsnew/2.0.rst
Expand Up @@ -166,6 +166,40 @@ New checkers

$ pylint a.py --load-plugins=pylint.extensions.emptystring

* A new extension was added, ``comparetozero.py`` which detects whenever
we compare integers to zero. This extension is disabled by default.
For instance, the examples below:

.. code-block:: python
if X != 0:
pass
if X == 0:
pass
can be written in a more natural way:

.. code-block:: python
if X:
pass
if not X:
pass
An exception to this is when zero is an allowed value whose meaning
is treated differently than ``None``. For example the meaning could be
``None`` means no limit, while ``0`` means the limit it zero!

You can activate this checker by adding the line::

load-plugins=pylint.extensions.comparetozero

to the ``MASTER`` section of your ``.pylintrc`` or using the command::

$ pylint a.py --load-plugins=pylint.extensions.comparetozero

* We've added new error conditions for ``bad-super-call`` which now detect
the usage of ``super(type(self), self)`` and ``super(self.__class__, self)``
patterns. These can lead to recursion loop in derived classes. The problem
Expand Down
71 changes: 71 additions & 0 deletions pylint/extensions/comparetozero.py
@@ -0,0 +1,71 @@
# -*- coding: utf-8 -*-
# Copyright (c) 2016 Alexander Todorov <atodorov@MrSenko.com>

# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
# For details: https://github.com/PyCQA/pylint/blob/master/COPYING

"""Looks for comparisons to empty string."""

import itertools

import astroid

from pylint import interfaces
from pylint import checkers
from pylint.checkers import utils


def _is_constant_zero(node):
return isinstance(node, astroid.Const) and node.value == 0


class CompareToZeroChecker(checkers.BaseChecker):
"""Checks for comparisons to zero.
Most of the times you should use the fact that integers with a value of 0 are false.
An exception to this rule is when 0 is allowed in the program and has a
different meaning than None!
"""

__implements__ = (interfaces.IAstroidChecker,)

# configuration section name
name = 'compare-to-zero'
msgs = {'C2001': ('Avoid comparisons to zero',
'compare-to-zero',
'Used when Pylint detects comparison to a 0 constant.'),
}

priority = -2
options = ()

@utils.check_messages('compare-to-zero')
def visit_compare(self, node):
_operators = ['!=', '==', 'is not', 'is']
# note: astroid.Compare has the left most operand in node.left
# while the rest are a list of tuples in node.ops
# the format of the tuple is ('compare operator sign', node)
# here we squash everything into `ops` to make it easier for processing later
ops = [('', node.left)]
ops.extend(node.ops)
ops = list(itertools.chain(*ops))

for ops_idx in range(len(ops) - 2):
op_1 = ops[ops_idx]
op_2 = ops[ops_idx + 1]
op_3 = ops[ops_idx + 2]
error_detected = False

# 0 ?? X
if _is_constant_zero(op_1) and op_2 in _operators + ['<']:
error_detected = True
# X ?? 0
elif op_2 in _operators + ['>'] and _is_constant_zero(op_3):
error_detected = True

if error_detected:
self.add_message('compare-to-zero', node=node)


def register(linter):
"""Required method to auto register this checker."""
linter.register_checker(CompareToZeroChecker(linter))
28 changes: 28 additions & 0 deletions pylint/test/extensions/data/compare_to_zero.py
@@ -0,0 +1,28 @@
# pylint: disable=literal-comparison,missing-docstring,misplaced-comparison-constant

X = 123
Y = len('test')

if X is 0: # [compare-to-zero]
pass

if Y is not 0: # [compare-to-zero]
pass

if X == 0: # [compare-to-zero]
pass

if Y != 0: # [compare-to-zero]
pass

if X > 0: # [compare-to-zero]
pass

if X < 0: # this is allowed
pass

if 0 < X: # [compare-to-zero]
pass

if 0 > X: # this is allowed
pass
56 changes: 56 additions & 0 deletions pylint/test/extensions/test_comparetozero.py
@@ -0,0 +1,56 @@
# Copyright (c) 2016 Alexander Todorov <atodorov@MrSenko.com>

# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
# For details: https://github.com/PyCQA/pylint/blob/master/COPYING

"""Tests for the pylint checker in :mod:`pylint.extensions.emptystring
"""

import os
import os.path as osp
import unittest

from pylint import checkers
from pylint.extensions.comparetozero import CompareToZeroChecker
from pylint.lint import PyLinter
from pylint.reporters import BaseReporter


class CompareToZeroTestReporter(BaseReporter):

def handle_message(self, msg):
self.messages.append(msg)

def on_set_current_module(self, module, filepath):
self.messages = []


class CompareToZeroUsedTC(unittest.TestCase):

@classmethod
def setUpClass(cls):
cls._linter = PyLinter()
cls._linter.set_reporter(CompareToZeroTestReporter())
checkers.initialize(cls._linter)
cls._linter.register_checker(CompareToZeroChecker(cls._linter))
cls._linter.disable('I')

def test_comparetozero_message(self):
elif_test = osp.join(osp.dirname(osp.abspath(__file__)), 'data',
'compare_to_zero.py')
self._linter.check([elif_test])
msgs = self._linter.reporter.messages
self.assertEqual(len(msgs), 6)
for msg in msgs:
self.assertEqual(msg.symbol, 'compare-to-zero')
self.assertEqual(msg.msg, 'Avoid comparisons to zero')
self.assertEqual(msgs[0].line, 6)
self.assertEqual(msgs[1].line, 9)
self.assertEqual(msgs[2].line, 12)
self.assertEqual(msgs[3].line, 15)
self.assertEqual(msgs[4].line, 18)
self.assertEqual(msgs[5].line, 24)


if __name__ == '__main__':
unittest.main()

0 comments on commit 6a8a69d

Please sign in to comment.