Add checker for unsubscriptable values used in subscript expression.

Fixes issue #561.

6 files changed, 144 insertions, 2 deletions

diff --git a/ChangeLog b/ChangeLog
index 948520c..2ee9c15 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,10 @@ ChangeLog for Pylint
 
 --
 
+    * Add new message, 'unsubscriptable-value', that is emitted when
+      value used in subscription expression doesn't support subscription
+      (i.e. doesn't define __getitem__ method).
+
     * Don't warn about abstract classes instantiated in their own
       body. Closes issue #627.
 
diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py
index a37a442..f8067ba 100644
--- a/pylint/checkers/typecheck.py
+++ b/pylint/checkers/typecheck.py
@@ -37,7 +37,8 @@ from pylint.checkers.utils import (
     is_super, check_messages, decorated_with_property,
     decorated_with, node_ignores_exception,
     is_iterable, is_mapping, supports_membership_test,
-    is_comprehension, is_inside_abstract_class)
+    is_comprehension, is_inside_abstract_class,
+    supports_subscript)
 from pylint import utils
 
 
@@ -153,6 +154,10 @@ MSGS = {
               'unsupported-membership-test',
               'Emitted when an instance in membership test expression doesn\'t'
               'implement membership protocol (__contains__/__iter__/__getitem__)'),
+    'E1136': ("Value '%s' is unsubscriptable",
+              'unsubscriptable-value',
+              "Emitted when a subscripted value doesn't support subscription"
+              "(i.e. doesn't define __getitem__ method)"),
     }
 
 # builtin sequence types in Python 2 and 3.
@@ -817,6 +822,23 @@ accessed. Python regular expressions are accepted.'}
         if operator in ['in', 'not in']:
             self._check_membership_test(right)
 
+    @check_messages('unsubscriptable-value')
+    def visit_subscript(self, node):
+        if isinstance(node.value, (astroid.ListComp, astroid.DictComp)):
+            return
+        if isinstance(node.value, astroid.SetComp):
+            self.add_message('unsubscriptable-value',
+                             args=node.value.as_string(),
+                             node=node.value)
+        infered = helpers.safe_infer(node.value)
+        if infered is None or infered is astroid.YES:
+            return
+        if not supports_subscript(infered):
+            self.add_message('unsubscriptable-value',
+                             args=node.value.as_string(),
+                             node=node.value)
+
+
 
 class IterableChecker(BaseChecker):
     """
diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py
index 5f9c227..a3c8c01 100644
--- a/pylint/checkers/utils.py
+++ b/pylint/checkers/utils.py
@@ -618,6 +618,10 @@ def _supports_iteration_protocol(value):
     return _hasattr(value, ITER_METHOD) or _hasattr(value, GETITEM_METHOD)
 
 
+def _supports_subscript_protocol(value):
+    return _hasattr(value, GETITEM_METHOD)
+
+
 def _is_abstract_class_name(name):
     lname = name.lower()
     is_mixin = lname.endswith('mixin')
@@ -687,6 +691,20 @@ def supports_membership_test(value):
     return is_iterable(value)
 
 
+def supports_subscript(value):
+    if isinstance(value, astroid.ClassDef):
+        if not helpers.has_known_bases(value):
+            return False
+        meta = value.metaclass()
+        if meta is not None and _supports_subscript_protocol(meta):
+            return True
+    if isinstance(value, astroid.Instance):
+        if not helpers.has_known_bases(value):
+            return True
+        if _supports_subscript_protocol(value):
+            return True
+    return False
+
 # TODO(cpopa): deprecate these or leave them as aliases?
 safe_infer = astroid.helpers.safe_infer
 has_known_bases = astroid.helpers.has_known_bases
diff --git a/pylint/test/functional/invalid_sequence_index.py b/pylint/test/functional/invalid_sequence_index.py
index 232b727..32b75f2 100644
--- a/pylint/test/functional/invalid_sequence_index.py
+++ b/pylint/test/functional/invalid_sequence_index.py
@@ -213,4 +213,4 @@ def function26():
         """Permit extslice syntax by implementing __getitem__"""
         def __getitem__(self, index):
             return 0
-    return ExtSliceTest[..., 0] # no error
+    return ExtSliceTest()[..., 0] # no error
diff --git a/pylint/test/functional/unsubscriptable_value.py b/pylint/test/functional/unsubscriptable_value.py
new file mode 100644
index 0000000..ed94244
--- /dev/null
+++ b/pylint/test/functional/unsubscriptable_value.py
@@ -0,0 +1,84 @@
+"""
+Checks that value used in a subscript supports subscription
+(i.e. defines __getitem__ method).
+"""
+# pylint: disable=missing-docstring,pointless-statement,expression-not-assigned
+# pylint: disable=too-few-public-methods,import-error,invalid-name
+import six
+
+# primitives
+numbers = [1, 2, 3]
+numbers[0]
+"123"[0]
+u"123"[0]
+b"123"[0]
+bytearray(b"123")[0]
+dict(a=1, b=2)['a']
+(1, 2, 3)[0]
+
+# list/dict comprehensions are fine
+[x for x in range(10)][0]
+{x: 10 - x for x in range(10)}[0]
+
+
+# instances
+class NonSubscriptable(object):
+    pass
+
+class Subscriptable(object):
+    def __getitem__(self, key):
+        return key + key
+
+NonSubscriptable()[0]  # [unsubscriptable-value]
+NonSubscriptable[0]  # [unsubscriptable-value]
+Subscriptable()[0]
+Subscriptable[0]  # [unsubscriptable-value]
+
+# generators are not subscriptable
+def powers_of_two():
+    k = 0
+    while k < 10:
+        yield 2 ** k
+        k += 1
+
+powers_of_two()[0]  # [unsubscriptable-value]
+powers_of_two[0]  # [unsubscriptable-value]
+
+
+# check that primitive non subscriptable types are catched
+True[0]  # [unsubscriptable-value]
+None[0]  # [unsubscriptable-value]
+8.5[0]  # [unsubscriptable-value]
+10[0]  # [unsubscriptable-value]
+
+# sets are not subscriptable
+{x ** 2 for x in range(10)}[0]  # [unsubscriptable-value]
+set(numbers)[0]  # [unsubscriptable-value]
+frozenset(numbers)[0]  # [unsubscriptable-value]
+
+# skip instances with unknown base classes
+from some_missing_module import LibSubscriptable
+
+class MaybeSubscriptable(LibSubscriptable):
+    pass
+
+MaybeSubscriptable()[0]
+
+# subscriptable classes (through metaclasses)
+
+class MetaSubscriptable(type):
+    def __getitem__(cls, key):
+        return key + key
+
+class SubscriptableClass(six.with_metaclass(MetaSubscriptable, object)):
+    pass
+
+SubscriptableClass[0]
+SubscriptableClass()[0]  # [unsubscriptable-value]
+
+# functions are not subscriptable
+def test(*args, **kwargs):
+    return args, kwargs
+
+test()[0]
+test[0]  # [unsubscriptable-value]
diff --git a/pylint/test/functional/unsubscriptable_value.txt b/pylint/test/functional/unsubscriptable_value.txt
new file mode 100644
index 0000000..d39c86a
--- /dev/null
+++ b/pylint/test/functional/unsubscriptable_value.txt
@@ -0,0 +1,14 @@
+unsubscriptable-value:32::Value 'NonSubscriptable()' is unsubscriptable
+unsubscriptable-value:33::Value 'NonSubscriptable' is unsubscriptable
+unsubscriptable-value:35::Value 'Subscriptable' is unsubscriptable
+unsubscriptable-value:44::Value 'powers_of_two()' is unsubscriptable
+unsubscriptable-value:45::Value 'powers_of_two' is unsubscriptable
+unsubscriptable-value:49::Value 'True' is unsubscriptable
+unsubscriptable-value:50::Value 'None' is unsubscriptable
+unsubscriptable-value:51::Value '8.5' is unsubscriptable
+unsubscriptable-value:52::Value '10' is unsubscriptable
+unsubscriptable-value:55::Value '{(x) ** (2) for x in range(10)}' is unsubscriptable
+unsubscriptable-value:56::Value 'set(numbers)' is unsubscriptable
+unsubscriptable-value:57::Value 'frozenset(numbers)' is unsubscriptable
+unsubscriptable-value:77::Value 'SubscriptableClass()' is unsubscriptable
+unsubscriptable-value:84::Value 'test' is unsubscriptable