Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make setters private (rework of #69) #107

Merged
merged 7 commits into from Mar 25, 2015
@@ -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']

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 22, 2015

Member

Make this a tuple instead (it shouldn't be mutable).


_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']

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 24, 2015

Member

This should be a tuple too.

is_public = True
_nest = staticmethod(lambda s: {'def': Function, 'class': Class}[s])
module = property(lambda self: self)
@@ -148,6 +150,10 @@ 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:
if decorator.name.startswith(self.name):

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 22, 2015

Member

Also check that after self.name there is a period, otherwise this will be considered private:

class Foo(object):
    @foobar
    def foo():
        pass

This comment has been minimized.

Copy link
@glennmatthews

glennmatthews Mar 23, 2015

Author Contributor

Great catch, thanks.

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 +169,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 +232,7 @@ def __call__(self, filelike, filename):
self.stream = TokenStream(StringIO(src))
self.filename = filename
self.all = None
self._decorators = []

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 22, 2015

Member

Maybe this should be named _current_decorators or something of this sort. At first I was confused as to why this is inside Parser.

This comment has been minimized.

Copy link
@glennmatthews

glennmatthews Mar 23, 2015

Author Contributor

Changed to _accumulated_decorators

return self.parse_module()

current = property(lambda self: self.stream.current)
@@ -254,13 +268,57 @@ def parse_docstring(self):
return docstring
return None

def parse_decorators(self):
"""Called after first @ is found.
Build list of current decorators and return last parsed token,

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 22, 2015

Member

This doesn't really return the last parsed token - the docstring should be updated.

which is 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']):
break
elif self.current.kind == tk.OP and self.current.value == '@':
# New decorator found.

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 22, 2015

Member

This comment confused me, because I didn't understand how a new decorator is appended if it's just been found out. State that you're saving the old one and resetting the parameters.

self._decorators.append(
Decorator(''.join(name), ''.join(arguments)))
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:
# Ignoe newlines
pass
else:
# Keep accumulating 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(
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 +382,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 +414,20 @@ def parse_definition(self, class_):
self.leapfrog(tk.INDENT)
assert self.current.kind != tk.INDENT
docstring = self.parse_docstring()
decorators = self._decorators
self._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)",
@@ -0,0 +1,242 @@
"""
Unit test for pep257 module decorator handling.

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 22, 2015

Member

Put the one-liner text in the same line as the opening triple-quotes.

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 22, 2015

Member

(Goes for the whole file)

Use tox or py.test to run the test suite.
"""

try:
from StringIO import StringIO
except ImportError:
from io import StringIO

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 = """
@first_decorator
class Foo:
pass
"""

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 22, 2015

Member

If you're using multi-line strings, indent them like the starting line and then use textwrap.dedent (it looks much better this way). You can look at test_pep257.py for examples.

This comment has been minimized.

Copy link
@glennmatthews

glennmatthews Mar 23, 2015

Author Contributor

Great suggestion, thanks.

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 they arguments.
"""
code = """
@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 = """
@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 = """
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):
"""
Method decorators are accumulated.
"""
code = """
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):
"""
It accumulates decorators for functions.
"""
code = """@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 = """
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 = 'class ClassName:\n def %s(self):\n' % (name)

module = pep257.Module(
'module_name',
source,
0,
1,
[],
'Docstring for module',
[],
None,
all,
)

This comment has been minimized.

Copy link
@Nurdok

Nurdok Mar 22, 2015

Member

This formatting is a bit weird... If you're putting one argument per line, at least use keyword arguments.

This comment has been minimized.

Copy link
@glennmatthews

glennmatthews Mar 23, 2015

Author Contributor

This was inherited from #69 - I agree it's atypical. Will clean it up.


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,
)

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
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.