Skip to content

Commit

Permalink
Extracted overlapping exceptions checker into extension
Browse files Browse the repository at this point in the history
  • Loading branch information
molobrakos authored and PCManticore committed Dec 15, 2016
1 parent 2fa1885 commit ea4273c
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 41 deletions.
26 changes: 1 addition & 25 deletions pylint/checkers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,6 @@ def _annotated_unpack_infer(stmt, context=None):
'Used when the exception to catch is of the form \
"except A or B:". If intending to catch multiple, \
rewrite as "except (A, B):"'),
'W0714': ('Overlapping exceptions (%s)',
'overlapping-except',
'Used when exceptions in handler overlap or are identical')
}


Expand Down Expand Up @@ -324,8 +321,7 @@ def _check_catching_non_exception(self, handler, exc, part):

@utils.check_messages('bare-except', 'broad-except',
'binary-op-exception', 'bad-except-order',
'catching-non-exception', 'duplicate-except',
'overlapping-except')
'catching-non-exception', 'duplicate-except')
def visit_tryexcept(self, node):
"""check for empty except"""
exceptions_classes = []
Expand All @@ -349,7 +345,6 @@ def visit_tryexcept(self, node):
except astroid.InferenceError:
continue

handled_in_clause = []
for part, exc in excs:
if exc is astroid.YES:
continue
Expand All @@ -366,25 +361,6 @@ def visit_tryexcept(self, node):
exc_ancestors = [anc for anc in exc.ancestors()
if isinstance(anc, astroid.ClassDef)]

for prev_part, prev_exc in handled_in_clause:
prev_exc_ancestors = [anc for anc in prev_exc.ancestors()
if isinstance(anc, astroid.ClassDef)]
if exc == prev_exc:
self.add_message('overlapping-except',
node=handler.type,
args='%s and %s are the same' %
(prev_part.as_string(),
part.as_string()))
elif (prev_exc in exc_ancestors or
exc in prev_exc_ancestors):
ancestor = part if exc in prev_exc_ancestors else prev_part
descendant = part if prev_exc in exc_ancestors else prev_part
self.add_message('overlapping-except',
node=handler.type,
args='%s is an ancestor class of %s' %
(ancestor.as_string(), descendant.as_string()))
handled_in_clause += [(part, exc)]

for previous_exc in exceptions_classes:
if previous_exc in exc_ancestors:
msg = '%s is an ancestor class of %s' % (
Expand Down
2 changes: 1 addition & 1 deletion pylint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def save_results(results, base):
try:
with open(data_file, _PICK_DUMP) as stream:
pickle.dump(results, stream)
except (IOError, OSError) as ex: # pylint: disable=overlapping-except
except (IOError, OSError) as ex:
print('Unable to create file %s: %s' % (data_file, ex), file=sys.stderr)


Expand Down
81 changes: 81 additions & 0 deletions pylint/extensions/overlapping_exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# -*- coding: utf-8 -*-

# 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 overlapping exceptions."""

import astroid

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

from pylint.checkers.exceptions import _annotated_unpack_infer


class OverlappingExceptionsChecker(checkers.BaseChecker):
"""Checks for two or more exceptions in the same exception handler
clause that are identical or parts of the same inheritence hierarchy
(i.e. overlapping)."""

__implements__ = interfaces.IAstroidChecker

name = 'overlap-except'
msgs = {'W0714': ('Overlapping exceptions (%s)',
'overlapping-except',
'Used when exceptions in handler overlap or are identical')}
priority = -2
options = ()

@utils.check_messages('overlapping-except')
def visit_tryexcept(self, node):
"""check for empty except"""
for _, handler in enumerate(node.handlers):
if handler.type is None:
continue
if isinstance(handler.type, astroid.BoolOp):
continue
try:
excs = list(_annotated_unpack_infer(handler.type))
except astroid.InferenceError:
continue

handled_in_clause = []
for part, exc in excs:
if exc is astroid.YES:
continue
if (isinstance(exc, astroid.Instance) and
utils.inherit_from_std_ex(exc)):
# pylint: disable=protected-access
exc = exc._proxied

if not isinstance(exc, astroid.ClassDef):
continue

exc_ancestors = [anc for anc in exc.ancestors()
if isinstance(anc, astroid.ClassDef)]

for prev_part, prev_exc in handled_in_clause:
prev_exc_ancestors = [anc for anc in prev_exc.ancestors()
if isinstance(anc, astroid.ClassDef)]
if exc == prev_exc:
self.add_message('overlapping-except',
node=handler.type,
args='%s and %s are the same' %
(prev_part.as_string(),
part.as_string()))
elif (prev_exc in exc_ancestors or
exc in prev_exc_ancestors):
ancestor = part if exc in prev_exc_ancestors else prev_part
descendant = part if prev_exc in exc_ancestors else prev_part
self.add_message('overlapping-except',
node=handler.type,
args='%s is an ancestor class of %s' %
(ancestor.as_string(), descendant.as_string()))
handled_in_clause += [(part, exc)]


def register(linter):
"""Required method to auto register this checker."""
linter.register_checker(OverlappingExceptionsChecker(linter))
82 changes: 82 additions & 0 deletions pylint/test/extensions/test_overlapping_exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# 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.overlapping_exceptions
"""

from sys import version_info
import os
from os.path import join, dirname
import unittest

from pylint import checkers
from pylint.lint import PyLinter
from pylint.reporters import BaseReporter
from pylint.extensions.overlapping_exceptions import OverlappingExceptionsChecker

class TestReporter(BaseReporter):

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

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


class CheckOverlappingExceptions(unittest.TestCase):

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

def test_overlapping_exceptions(self):
test = join(dirname(__file__), 'data', 'overlapping_exceptions.py')
self._linter.check([test])
msgs = self._linter.reporter.messages

expected = [
(13, 'Overlapping exceptions (SomeException and SomeException are the same)'),
(18, 'Overlapping exceptions (SomeException is an ancestor class of SubclassException)'),
(23, 'Overlapping exceptions (SomeException and AliasException are the same)'),
(28, 'Overlapping exceptions (AliasException is an ancestor class of SubclassException)'),
(34, 'Overlapping exceptions (SomeException and AliasException are the same)'),
(34, 'Overlapping exceptions (SomeException is an ancestor class of SubclassException)'),
(34, 'Overlapping exceptions (AliasException is an ancestor class of SubclassException)'),
(39, 'Overlapping exceptions (ArithmeticError is an ancestor class of FloatingPointError)'),
(44, 'Overlapping exceptions (ValueError is an ancestor class of UnicodeDecodeError)')
]

self.assertEqual(len(msgs), len(expected))
for msg, exp in zip(msgs, expected):
self.assertEqual(msg.msg_id, 'W0714')
self.assertEqual(msg.symbol, 'overlapping-except')
self.assertEqual(msg.category, 'warning')
self.assertEqual((msg.line, msg.msg), exp)

@unittest.skipIf(version_info < (3, 3), "not relevant to Python version")
def test_overlapping_exceptions_py33(self):
"""From Python 3.3 both IOError and socket.error are aliases for OSError."""
test = join(dirname(__file__), 'data', 'overlapping_exceptions_py33.py')
self._linter.check([test])
msgs = self._linter.reporter.messages

expected = [
(7, 'Overlapping exceptions (IOError and OSError are the same)'),
(12, 'Overlapping exceptions (socket.error and OSError are the same)'),
(17, 'Overlapping exceptions (socket.error is an ancestor class of ConnectionError)'),
]

self.assertEqual(len(msgs), len(expected))
for msg, exp in zip(msgs, expected):
self.assertEqual(msg.msg_id, 'W0714')
self.assertEqual(msg.symbol, 'overlapping-except')
self.assertEqual(msg.category, 'warning')
self.assertEqual((msg.line, msg.msg), exp)


if __name__ == '__main__':
unittest.main()
2 changes: 1 addition & 1 deletion pylint/test/functional/invalid_exceptions_caught.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# pylint: disable=missing-docstring, too-few-public-methods
# pylint: disable=too-many-ancestors, no-absolute-import, import-error, multiple-imports,wrong-import-position, overlapping-except
# pylint: disable=too-many-ancestors, no-absolute-import, import-error, multiple-imports,wrong-import-position
from __future__ import print_function

import socket, binascii, abc, six
Expand Down
9 changes: 0 additions & 9 deletions pylint/test/functional/overlapping_exceptions.txt

This file was deleted.

2 changes: 0 additions & 2 deletions pylint/test/functional/overlapping_exceptions_py33.rc

This file was deleted.

3 changes: 0 additions & 3 deletions pylint/test/functional/overlapping_exceptions_py33.txt

This file was deleted.

0 comments on commit ea4273c

Please sign in to comment.