Address code review comments from @Nurdok.

Author: glennmatthews

pep257.py

@@ -94,8 +94,8 @@ def __repr__(self):
 
 class Definition(Value):
 
-    _fields = ['name', '_source', 'start', 'end', 'decorators', 'docstring',
-               'children', 'parent']
+    _fields = ('name', '_source', 'start', 'end', 'decorators', 'docstring',
+               'children', 'parent')
 
     _human = property(lambda self: humanize(type(self).__name__))
     kind = property(lambda self: self._human.split()[-1])
@@ -152,7 +152,8 @@ class Method(Function):
     def is_public(self):
         # Check if we are a setter/deleter method, and mark as private if so.
         for decorator in self.decorators:
-            if decorator.name.startswith(self.name):
+            # Given 'foo', match 'foo.bar' but not 'foobar' or 'sfoo'
+            if re(r"^{0}\.".format(self.name)).match(decorator.name):
                 return False
         name_is_public = not self.name.startswith('_') or is_magic(self.name)
         return self.parent.is_public and name_is_public
@@ -232,7 +233,7 @@ def __call__(self, filelike, filename):
         self.stream = TokenStream(StringIO(src))
         self.filename = filename
         self.all = None
-        self._decorators = []
+        self._accumulated_decorators = []
         return self.parse_module()
 
     current = property(lambda self: self.stream.current)
@@ -271,8 +272,8 @@ def parse_docstring(self):
     def parse_decorators(self):
         """Called after first @ is found.
 
-        Build list of current decorators and return last parsed token,
-        which is def or class start token.
+        Parse decorators into self._accumulated_decorators.
+        Continue to do so until encountering the 'def' or 'class' start token.
         """
         name = []
         arguments = []
@@ -281,11 +282,13 @@ def parse_decorators(self):
         while self.current is not None:
             if (self.current.kind == tk.NAME and
                     self.current.value in ['def', 'class']):
+                # Done with decorators - found function or class proper
                 break
             elif self.current.kind == tk.OP and self.current.value == '@':
-                # New decorator found.
-                self._decorators.append(
+                # New decorator found. Store the decorator accumulated so far:
+                self._accumulated_decorators.append(
                     Decorator(''.join(name), ''.join(arguments)))
+                # Now reset to begin accumulating the new decorator:
                 name = []
                 arguments = []
                 at_arguments = False
@@ -295,18 +298,18 @@ def parse_decorators(self):
                 # Ignore close parenthesis
                 pass
             elif self.current.kind == tk.NEWLINE or self.current.kind == tk.NL:
-                # Ignoe newlines
+                # Ignore newlines
                 pass
             else:
-                # Keep accumulating decorator's name or argument.
+                # Keep accumulating current decorator's name or argument.
                 if not at_arguments:
                     name.append(self.current.value)
                 else:
                     arguments.append(self.current.value)
             self.stream.move()
 
         # Add decorator accumulated so far
-        self._decorators.append(
+        self._accumulated_decorators.append(
             Decorator(''.join(name), ''.join(arguments)))
 
     def parse_definitions(self, class_, all=False):
@@ -414,8 +417,8 @@ def parse_definition(self, class_):
             self.leapfrog(tk.INDENT)
             assert self.current.kind != tk.INDENT
             docstring = self.parse_docstring()
-            decorators = self._decorators
-            self._decorators = []
+            decorators = self._accumulated_decorators
+            self._accumulated_decorators = []
             log.debug("parsing nested defintions.")
             children = list(self.parse_definitions(class_))
             log.debug("finished parsing nested defintions for '%s'", name)

test_decorators.py

@@ -1,5 +1,4 @@
-"""
-Unit test for pep257 module decorator handling.
+"""Unit test for pep257 module decorator handling.
 
 Use tox or py.test to run the test suite.
 """
@@ -9,23 +8,21 @@
 except ImportError:
     from io import StringIO
 
+import textwrap
+
 import pep257
 
 
 class TestParser:
-    """
-    Check parsing of Python source code.
-    """
+    """Check parsing of Python source code."""
 
     def test_parse_class_single_decorator(self):
-        """
-        Class decorator is recorded in class instance.
-        """
-        code = """
-@first_decorator
-class Foo:
-    pass
-        """
+        """Class decorator is recorded in class instance."""
+        code = textwrap.dedent("""\
+            @first_decorator
+            class Foo:
+                pass
+        """)
         module = pep257.parse(StringIO(code), 'dummy.py')
         decorators = module.children[0].decorators
 
@@ -34,19 +31,17 @@ class Foo:
         assert '' == decorators[0].arguments
 
     def test_parse_class_decorators(self):
-        """
-        Class decorators are accumulated, together with they arguments.
-        """
-        code = """
-@first_decorator
-@second.decorator(argument)
-@third.multi.line(
-    decorator,
-    key=value,
-    )
-class Foo:
-    pass
-        """
+        """Class decorators are accumulated together with their arguments."""
+        code = textwrap.dedent("""\
+            @first_decorator
+            @second.decorator(argument)
+            @third.multi.line(
+                decorator,
+                key=value,
+                )
+            class Foo:
+                pass
+        """)
 
         module = pep257.parse(StringIO(code), 'dummy.py')
         defined_class = module.children[0]
@@ -61,17 +56,15 @@ class Foo:
         assert 'decorator,key=value,' == decorators[2].arguments
 
     def test_parse_class_nested_decorator(self):
-        """
-        Class decorator is recorded even for nested classes.
-        """
-        code = """
-@parent_decorator
-class Foo:
-    pass
-    @first_decorator
-    class NestedClass:
-        pass
-        """
+        """Class decorator is recorded even for nested classes."""
+        code = textwrap.dedent("""\
+            @parent_decorator
+            class Foo:
+                pass
+                @first_decorator
+                class NestedClass:
+                    pass
+        """)
         module = pep257.parse(StringIO(code), 'dummy.py')
         nested_class = module.children[0].children[0]
         decorators = nested_class.decorators
@@ -81,15 +74,13 @@ class NestedClass:
         assert '' == decorators[0].arguments
 
     def test_parse_method_single_decorator(self):
-        """
-        Method decorators are accumulated.
-        """
-        code = """
-class Foo:
-    @first_decorator
-    def method(self):
-        pass
-        """
+        """Method decorators are accumulated."""
+        code = textwrap.dedent("""\
+            class Foo:
+                @first_decorator
+                def method(self):
+                    pass
+        """)
 
         module = pep257.parse(StringIO(code), 'dummy.py')
         defined_class = module.children[0]
@@ -100,20 +91,18 @@ def method(self):
         assert '' == decorators[0].arguments
 
     def test_parse_method_decorators(self):
-        """
-        Method decorators are accumulated.
-        """
-        code = """
-class Foo:
-    @first_decorator
-    @second.decorator(argument)
-    @third.multi.line(
-        decorator,
-        key=value,
-        )
-    def method(self):
-        pass
-        """
+        """Multiple method decorators are accumulated along with their args."""
+        code = textwrap.dedent("""\
+            class Foo:
+                @first_decorator
+                @second.decorator(argument)
+                @third.multi.line(
+                    decorator,
+                    key=value,
+                    )
+                def method(self):
+                    pass
+        """)
 
         module = pep257.parse(StringIO(code), 'dummy.py')
         defined_class = module.children[0]
@@ -128,13 +117,12 @@ def method(self):
         assert 'decorator,key=value,' == decorators[2].arguments
 
     def test_parse_function_decorator(self):
-        """
-        It accumulates decorators for functions.
-        """
-        code = """@first_decorator
-def some_method(self):
-    pass
-        """
+        """A function decorator is also accumulated."""
+        code = textwrap.dedent("""\
+            @first_decorator
+            def some_method(self):
+                pass
+        """)
 
         module = pep257.parse(StringIO(code), 'dummy.py')
         decorators = module.children[0].decorators
@@ -144,17 +132,15 @@ def some_method(self):
         assert '' == decorators[0].arguments
 
     def test_parse_method_nested_decorator(self):
-        """
-        Method decorators are accumulated for nested methods.
-        """
-        code = """
-class Foo:
-    @parent_decorator
-    def method(self):
-        @first_decorator
-        def nested_method(arg):
-            pass
-        """
+        """Method decorators are accumulated for nested methods."""
+        code = textwrap.dedent("""\
+            class Foo:
+                @parent_decorator
+                def method(self):
+                    @first_decorator
+                    def nested_method(arg):
+                        pass
+        """)
 
         module = pep257.parse(StringIO(code), 'dummy.py')
         defined_class = module.children[0]
@@ -166,53 +152,25 @@ def nested_method(arg):
 
 
 class TestMethod:
-    """
-    Unit test for Method class.
-    """
+    """Unit test for Method class."""
 
     def makeMethod(self, name='someMethodName'):
-        """
-        Return a simple method instance.
-        """
+        """Return a simple method instance."""
         children = []
         all = ['ClassName']
-        source = 'class ClassName:\n def %s(self):\n' % (name)
-
-        module = pep257.Module(
-            'module_name',
-            source,
-            0,
-            1,
-            [],
-            'Docstring for module',
-            [],
-            None,
-            all,
-        )
-
-        parent = pep257.Class(
-            'ClassName',
-            source,
-            0,
-            1,
-            [],
-            'Docstring for class',
-            children,
-            module,
-            all,
-        )
-
-        return pep257.Method(
-            name,
-            source,
-            0,
-            1,
-            [],
-            'Docstring for method',
-            children,
-            parent,
-            all,
-        )
+        source = textwrap.dedent("""\
+            class ClassName:
+                def %s(self):
+        """ % (name))
+
+        module = pep257.Module('module_name', source, 0, 1, [],
+                               'Docstring for module', [], None, all)
+
+        cls = pep257.Class('ClassName', source, 0, 1, [],
+                           'Docstring for class', children, module, all)
+
+        return pep257.Method(name, source, 0, 1, [],
+                             'Docstring for method', children, cls, all)
 
     def test_is_public_normal(self):
         """Methods are normally public, even if decorated."""
@@ -240,3 +198,13 @@ def test_is_public_deleter(self):
         ]
 
         assert not method.is_public
+
+    def test_is_public_trick(self):
+        """Common prefix does not necessarily indicate private."""
+        method = self.makeMethod("foo")
+        method.decorators = [
+            pep257.Decorator('foobar', []),
+            pep257.Decorator('foobar.baz', []),
+        ]
+
+        assert method.is_public