Merge pull request #107 from glennmatthews/master

Make setters private (rework of #69)

Author: Nurdok

docs/release_notes.rst

@@ -5,6 +5,10 @@ Release Notes
 Current Development Version
 ---------------------------
 
+Bug Fixes
+
+* Property setter and deleter methods are now treated as private and do not
+  require docstrings separate from the main property method (#69, #107).
 
 0.5.0 - March 14th, 2015
 ------------------------

pep257.py

@@ -94,7 +94,8 @@ def __repr__(self):
 
 class Definition(Value):
 
-    _fields = 'name _source start end docstring children parent'.split()
+    _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])
@@ -116,7 +117,8 @@ def __str__(self):
 
 class Module(Definition):
 
-    _fields = 'name _source start end docstring children parent _all'.split()
+    _fields = ('name', '_source', 'start', 'end', 'decorators', 'docstring',
+               'children', 'parent', '_all')
     is_public = True
     _nest = staticmethod(lambda s: {'def': Function, 'class': Class}[s])
     module = property(lambda self: self)
@@ -148,6 +150,11 @@ class Method(Function):
 
     @property
     def is_public(self):
+        # Check if we are a setter/deleter method, and mark as private if so.
+        for decorator in self.decorators:
+            # 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
 
@@ -163,6 +170,13 @@ class NestedClass(Class):
     is_public = False
 
 
+class Decorator(Value):
+
+    """A decorator for function, method or class."""
+
+    _fields = 'name arguments'.split()
+
+
 class TokenKind(int):
     def __repr__(self):
         return "tk.{}".format(tk.tok_name[self])
@@ -219,6 +233,7 @@ def __call__(self, filelike, filename):
         self.stream = TokenStream(StringIO(src))
         self.filename = filename
         self.all = None
+        self._accumulated_decorators = []
         return self.parse_module()
 
     current = property(lambda self: self.stream.current)
@@ -254,13 +269,59 @@ def parse_docstring(self):
             return docstring
         return None
 
+    def parse_decorators(self):
+        """Called after first @ is found.
+
+        Parse decorators into self._accumulated_decorators.
+        Continue to do so until encountering the 'def' or 'class' start token.
+        """
+        name = []
+        arguments = []
+        at_arguments = False
+
+        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. 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
+            elif self.current.kind == tk.OP and self.current.value == '(':
+                at_arguments = True
+            elif self.current.kind == tk.OP and self.current.value == ')':
+                # Ignore close parenthesis
+                pass
+            elif self.current.kind == tk.NEWLINE or self.current.kind == tk.NL:
+                # Ignore newlines
+                pass
+            else:
+                # 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._accumulated_decorators.append(
+            Decorator(''.join(name), ''.join(arguments)))
+
     def parse_definitions(self, class_, all=False):
         """Parse multiple defintions and yield them."""
         while self.current is not None:
             log.debug("parsing defintion list, current token is %r (%s)",
                       self.current.kind, self.current.value)
             if all and self.current.value == '__all__':
                 self.parse_all()
+            elif self.current.kind == tk.OP and self.current.value == '@':
+                self.consume(tk.OP)
+                self.parse_decorators()
             elif self.current.value in ['def', 'class']:
                 yield self.parse_definition(class_._nest(self.current.value))
             elif self.current.kind == tk.INDENT:
@@ -324,7 +385,7 @@ def parse_module(self):
         assert self.current is None, self.current
         end = self.line
         module = Module(self.filename, self.source, start, end,
-                        docstring, children, None, self.all)
+                        [], docstring, children, None, self.all)
         for child in module.children:
             child.parent = module
         log.debug("finished parsing module.")
@@ -356,17 +417,20 @@ def parse_definition(self, class_):
             self.leapfrog(tk.INDENT)
             assert self.current.kind != tk.INDENT
             docstring = self.parse_docstring()
+            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)
             end = self.line - 1
         else:  # one-liner definition
             docstring = self.parse_docstring()
+            decorators = []  # TODO
             children = []
             end = self.line
             self.leapfrog(tk.NEWLINE)
         definition = class_(name, self.source, start, end,
-                            docstring, children, None)
+                            decorators, docstring, children, None)
         for child in definition.children:
             child.parent = definition
         log.debug("finished parsing %s '%s'. Next token is %r (%s)",

test_decorators.py

@@ -0,0 +1,210 @@
+"""Unit test for pep257 module decorator handling.
+
+Use tox or py.test to run the test suite.
+"""
+
+try:
+    from StringIO import StringIO
+except ImportError:
+    from io import StringIO
+
+import textwrap
+
+import pep257
+
+
+class TestParser:
+    """Check parsing of Python source code."""
+
+    def test_parse_class_single_decorator(self):
+        """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
+
+        assert 1 == len(decorators)
+        assert 'first_decorator' == decorators[0].name
+        assert '' == decorators[0].arguments
+
+    def test_parse_class_decorators(self):
+        """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]
+        decorators = defined_class.decorators
+
+        assert 3 == len(decorators)
+        assert 'first_decorator' == decorators[0].name
+        assert '' == decorators[0].arguments
+        assert 'second.decorator' == decorators[1].name
+        assert 'argument' == decorators[1].arguments
+        assert 'third.multi.line' == decorators[2].name
+        assert 'decorator,key=value,' == decorators[2].arguments
+
+    def test_parse_class_nested_decorator(self):
+        """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
+
+        assert 1 == len(decorators)
+        assert 'first_decorator' == decorators[0].name
+        assert '' == decorators[0].arguments
+
+    def test_parse_method_single_decorator(self):
+        """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]
+        decorators = defined_class.children[0].decorators
+
+        assert 1 == len(decorators)
+        assert 'first_decorator' == decorators[0].name
+        assert '' == decorators[0].arguments
+
+    def test_parse_method_decorators(self):
+        """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]
+        decorators = defined_class.children[0].decorators
+
+        assert 3 == len(decorators)
+        assert 'first_decorator' == decorators[0].name
+        assert '' == decorators[0].arguments
+        assert 'second.decorator' == decorators[1].name
+        assert 'argument' == decorators[1].arguments
+        assert 'third.multi.line' == decorators[2].name
+        assert 'decorator,key=value,' == decorators[2].arguments
+
+    def test_parse_function_decorator(self):
+        """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
+
+        assert 1 == len(decorators)
+        assert 'first_decorator' == decorators[0].name
+        assert '' == decorators[0].arguments
+
+    def test_parse_method_nested_decorator(self):
+        """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]
+        decorators = defined_class.children[0].children[0].decorators
+
+        assert 1 == len(decorators)
+        assert 'first_decorator' == decorators[0].name
+        assert '' == decorators[0].arguments
+
+
+class TestMethod:
+    """Unit test for Method class."""
+
+    def makeMethod(self, name='someMethodName'):
+        """Return a simple method instance."""
+        children = []
+        all = ['ClassName']
+        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."""
+        method = self.makeMethod('methodName')
+        method.decorators = [pep257.Decorator('some_decorator', [])]
+
+        assert method.is_public
+
+    def test_is_public_setter(self):
+        """Setter methods are considered private."""
+        method = self.makeMethod('methodName')
+        method.decorators = [
+            pep257.Decorator('some_decorator', []),
+            pep257.Decorator('methodName.setter', []),
+        ]
+
+        assert not method.is_public
+
+    def test_is_public_deleter(self):
+        """Deleter methods are also considered private."""
+        method = self.makeMethod('methodName')
+        method.decorators = [
+            pep257.Decorator('methodName.deleter', []),
+            pep257.Decorator('another_decorator', []),
+        ]
+
+        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