Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Exempt properties from D401 #546

Merged
merged 15 commits into from Aug 15, 2021
3 changes: 3 additions & 0 deletions docs/release_notes.rst
Expand Up @@ -8,6 +8,9 @@ Release Notes
Current Development Version
---------------------------

Bug Fixes

* Properties no longer cause D401
samj1912 marked this conversation as resolved.
Show resolved Hide resolved

6.1.1 - May 17th, 2021
---------------------------
Expand Down
1 change: 1 addition & 0 deletions docs/snippets/config.rst
Expand Up @@ -44,6 +44,7 @@ Available options are:
* ``match``
* ``match_dir``
* ``ignore_decorators``
* ``property_decorators``

See the :ref:`cli_usage` section for more information.

Expand Down
17 changes: 15 additions & 2 deletions src/pydocstyle/checker.py
Expand Up @@ -131,8 +131,12 @@ def check_source(
source,
filename,
ignore_decorators=None,
property_decorators=None,
ignore_inline_noqa=False,
):
self.property_decorators = (
{} if property_decorators is None else property_decorators
)
module = parse(StringIO(source), filename)
for definition in module:
for this_check in self.checks:
Expand Down Expand Up @@ -500,7 +504,11 @@ def check_imperative_mood(self, function, docstring): # def context
"Returns the pathname ...".

"""
if docstring and not function.is_test:
if (
docstring
and not function.is_test
and not function.is_property(self.property_decorators)
):
stripped = ast.literal_eval(docstring).strip()
if stripped:
first_word = strip_non_alphanumeric(stripped.split()[0])
Expand Down Expand Up @@ -1040,6 +1048,7 @@ def check(
select=None,
ignore=None,
ignore_decorators=None,
property_decorators=None,
ignore_inline_noqa=False,
):
"""Generate docstring errors that exist in `filenames` iterable.
Expand Down Expand Up @@ -1092,7 +1101,11 @@ def check(
with tk.open(filename) as file:
source = file.read()
for error in ConventionChecker().check_source(
source, filename, ignore_decorators, ignore_inline_noqa
source,
filename,
ignore_decorators,
property_decorators,
ignore_inline_noqa,
):
code = getattr(error, 'code', None)
if code in checked_codes:
Expand Down
2 changes: 2 additions & 0 deletions src/pydocstyle/cli.py
Expand Up @@ -42,12 +42,14 @@ def run_pydocstyle():
filename,
checked_codes,
ignore_decorators,
property_decorators,
) in conf.get_files_to_check():
errors.extend(
check(
(filename,),
select=checked_codes,
ignore_decorators=ignore_decorators,
property_decorators=property_decorators,
)
)
except IllegalConfiguration as error:
Expand Down
57 changes: 52 additions & 5 deletions src/pydocstyle/config.py
Expand Up @@ -186,6 +186,7 @@ class ConfigurationParser:
DEFAULT_MATCH_RE = r'(?!test_).*\.py'
DEFAULT_MATCH_DIR_RE = r'[^\.].*'
DEFAULT_IGNORE_DECORATORS_RE = ''
DEFAULT_PROPERTY_DECORATORS = "property,cached_property"
DEFAULT_CONVENTION = conventions.pep257

PROJECT_CONFIG_FILES = (
Expand Down Expand Up @@ -266,12 +267,21 @@ def _get_ignore_decorators(conf):
re(conf.ignore_decorators) if conf.ignore_decorators else None
)

def _get_property_decorators(conf):
"""Return the `property_decorators` as None or set."""
return (
set(conf.property_decorators.split(","))
if conf.proprety_decorators
else None
)

for name in self._arguments:
if os.path.isdir(name):
for root, dirs, filenames in os.walk(name):
config = self._get_config(os.path.abspath(root))
match, match_dir = _get_matches(config)
ignore_decorators = _get_ignore_decorators(config)
property_decorators = _get_property_decorators(config)

# Skip any dirs that do not match match_dir
dirs[:] = [d for d in dirs if match_dir(d)]
Expand All @@ -283,13 +293,20 @@ def _get_ignore_decorators(conf):
full_path,
list(config.checked_codes),
ignore_decorators,
property_decorators,
)
else:
config = self._get_config(os.path.abspath(name))
match, _ = _get_matches(config)
ignore_decorators = _get_ignore_decorators(config)
property_decorators = _get_property_decorators(config)
if match(name):
yield (name, list(config.checked_codes), ignore_decorators)
yield (
name,
list(config.checked_codes),
ignore_decorators,
property_decorators,
)

# --------------------------- Private Methods -----------------------------

Expand Down Expand Up @@ -485,7 +502,12 @@ def _merge_configuration(self, parent_config, child_options):
self._set_add_options(error_codes, child_options)

kwargs = dict(checked_codes=error_codes)
for key in ('match', 'match_dir', 'ignore_decorators'):
for key in (
'match',
'match_dir',
'ignore_decorators',
'property_decorators',
):
kwargs[key] = getattr(child_options, key) or getattr(
parent_config, key
)
Expand Down Expand Up @@ -519,9 +541,15 @@ def _create_check_config(cls, options, use_defaults=True):
checked_codes = cls._get_checked_errors(options)

kwargs = dict(checked_codes=checked_codes)
for key in ('match', 'match_dir', 'ignore_decorators'):
defaults = {
'match': "DEFAULT_MATCH_RE",
'match_dir': "MATCH_DIR_RE",
'ignore_decorators': "IGNORE_DECORATORS_RE",
'property_decorators': "PROPERTY_DECORATORS",
}
for key, default in defaults.items():
kwargs[key] = (
getattr(cls, f'DEFAULT_{key.upper()}_RE')
getattr(cls, f"DEFAULT_{default}")
if getattr(options, key) is None and use_defaults
else getattr(options, key)
)
Expand Down Expand Up @@ -855,14 +883,33 @@ def _create_option_parser(cls):
)
),
)
option(
'--property-decorators',
metavar='<property-decorators>',
default=None,
help=(
"consider any method decorated with one of these "
"decorators as a property, and consequently allow "
"a docstring which is not in imperative mood; default "
"is --property-decorators='{}'".format(
cls.DEFAULT_PROPERTY_DECORATORS
)
),
)

return parser


# Check configuration - used by the ConfigurationParser class.
CheckConfiguration = namedtuple(
'CheckConfiguration',
('checked_codes', 'match', 'match_dir', 'ignore_decorators'),
(
'checked_codes',
'match',
'match_dir',
'ignore_decorators',
'property_decorators',
),
)


Expand Down
7 changes: 7 additions & 0 deletions src/pydocstyle/parser.py
Expand Up @@ -218,6 +218,13 @@ def is_overload(self):
return True
return False

def is_property(self, decorators):
"""Return True iff the method decorated with property."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This says iff which tends to only mean "if and only if" but that's not actually true here because anything with the string "property" in the name will return True, not just the @property built-in decorator

for decorator in self.decorators:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having a hard time understanding what the difference is between self.decorators and the argument decorators. Can these names be more descriptive?

Also couldn't this be something like

return any(decorator.name in decorators for decorator in self.decorators)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve improved this. I also changed the above is_overload to match.

if decorator.name in decorators:
return True
return False

@property
def is_test(self):
"""Return True if this function is a test function/method.
Expand Down
5 changes: 5 additions & 0 deletions src/tests/test_cases/test.py
Expand Up @@ -42,6 +42,11 @@ def overloaded_method(a):
"D418: Function/ Method decorated with @overload"
" shouldn't contain a docstring")

@property
def foo(self):
"""The foo of the thing, which isn't in imperitive mood."""
return "hello"

@expect('D102: Missing docstring in public method')
def __new__(self=None):
pass
Expand Down