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

Trigger a warning when several docstrings are detected #802

Merged
merged 4 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ What's New?
in development
^^^^^^^^^^^^^^

* Trigger a warning when several docstrings are detected for the same object.

pydoctor 24.3.3
^^^^^^^^^^^^^^^

Expand Down
6 changes: 2 additions & 4 deletions pydoctor/astbuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ def warn(msg: str) -> None:
return

if obj is not None:
obj.docstring = docstring
obj._setDocstringValue(docstring, expr.lineno)
# TODO: It might be better to not perform docstring parsing until
# we have the final docstrings for all objects.
obj.parsed_docstring = None
Expand Down Expand Up @@ -959,9 +959,7 @@ def _handlePropertyDef(self,
if tag == 'return':
if not pdoc.has_body:
pdoc = field.body()
# Avoid format_summary() going back to the original
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a drive-by cleanup. We don’t generated the summary from the docstring source anymore, so ne don’t need this hack.

# empty-body docstring.
attr.docstring = ''

elif tag == 'rtype':
attr.parsed_type = field.body()
else:
Expand Down
13 changes: 13 additions & 0 deletions pydoctor/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,21 @@ def setup(self) -> None:

def setDocstring(self, node: astutils.Str) -> None:
lineno, doc = astutils.extract_docstring(node)
self._setDocstringValue(doc, lineno)

def _setDocstringValue(self, doc:str, lineno:int) -> None:
if self.docstring or self.parsed_docstring: # some object have a parsed docstring only like the ones coming from ivar fields
msg = 'Existing docstring'
if self.docstring_lineno:
msg += f' at line {self.docstring_lineno}'
msg += ' is overriden'
self.report(msg, 'docstring', lineno_offset=lineno-self.docstring_lineno)
self.docstring = doc
self.docstring_lineno = lineno
# Due to the current process for parsing doc strings, some objects might already have a parsed_docstring populated at this moment.
# This is an unfortunate behaviour but it’s too big of a refactor for now (see https://github.com/twisted/pydoctor/issues/798).
if self.parsed_docstring:
tristanlatr marked this conversation as resolved.
Show resolved Hide resolved
self.parsed_docstring = None

def setLineNumber(self, lineno: LineFromDocstringField | LineFromAst | int) -> None:
"""
Expand Down
80 changes: 63 additions & 17 deletions pydoctor/test/test_astbuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -1114,11 +1114,11 @@ def method1():
def method2():
pass

method1.__doc__ = "Updated docstring #1"
method1.__doc__ = "Override docstring #1"

fun.__doc__ = "Happy Happy Joy Joy"
CLS.__doc__ = "Clears the screen"
CLS.method2.__doc__ = "Updated docstring #2"
CLS.method2.__doc__ = "Set docstring #2"

None.__doc__ = "Free lunch!"
real.__doc__ = "Second breakfast"
Expand All @@ -1137,24 +1137,19 @@ def mark_unavailable(func):
assert CLS.docstring == """Clears the screen"""
method1 = CLS.contents['method1']
assert method1.kind is model.DocumentableKind.METHOD
assert method1.docstring == "Updated docstring #1"
assert method1.docstring == "Override docstring #1"
method2 = CLS.contents['method2']
assert method2.kind is model.DocumentableKind.METHOD
assert method2.docstring == "Updated docstring #2"
assert method2.docstring == "Set docstring #2"
captured = capsys.readouterr()
lines = captured.out.split('\n')
assert len(lines) > 0 and lines[0] == \
"<test>:20: Unable to figure out target for __doc__ assignment"
assert len(lines) > 1 and lines[1] == \
"<test>:21: Unable to figure out target for __doc__ assignment: " \
"computed full name not found: real"
assert len(lines) > 2 and lines[2] == \
"<test>:22: Unable to figure out value for __doc__ assignment, " \
"maybe too complex"
assert len(lines) > 3 and lines[3] == \
"<test>:23: Ignoring value assigned to __doc__: not a string"
assert len(lines) == 5 and lines[-1] == ''

assert captured.out == (
'<test>:14: Existing docstring at line 8 is overriden\n'
'<test>:20: Unable to figure out target for __doc__ assignment\n'
'<test>:21: Unable to figure out target for __doc__ assignment: computed full name not found: real\n'
'<test>:22: Unable to figure out value for __doc__ assignment, maybe too complex\n'
'<test>:23: Ignoring value assigned to __doc__: not a string\n'
)

@systemcls_param
def test_docstring_assignment_detuple(systemcls: Type[model.System], capsys: CapSys) -> None:
"""We currently don't trace values for detupling assignments, so when
Expand Down Expand Up @@ -2747,3 +2742,54 @@ def test_typealias_unstring(systemcls: Type[model.System]) -> None:
# there is not Constant nodes in the type alias anymore
next(n for n in ast.walk(typealias.value) if isinstance(n, ast.Constant))

@systemcls_param
def test_mutilple_docstrings_warnings(systemcls: Type[model.System], capsys: CapSys) -> None:
"""
When pydoctor encounters multiple places where the docstring is defined, it reports a warning.
"""
src = '''
class C:
a: int;"docs"
def _(self):
self.a = 0; "re-docs"

class B:
"""
@ivar a: docs
"""
a: int
"re-docs"

class A:
"""docs"""
A.__doc__ = 're-docs'
'''
fromText(src, systemcls=systemcls)
assert capsys.readouterr().out == ('<test>:5: Existing docstring at line 3 is overriden\n'
'<test>:12: Existing docstring at line 9 is overriden\n'
'<test>:16: Existing docstring at line 15 is overriden\n')

@systemcls_param
def test_mutilple_docstring_with_doc_comments_warnings(systemcls: Type[model.System], capsys: CapSys) -> None:
src = '''
class C:
a: int;"docs" #: re-docs

class B:
"""
@ivar a: docs
"""
#: re-docs
a: int

class B2:
"""
@ivar a: docs
"""
#: re-docs
a: int
"re-re-docs"
'''
fromText(src, systemcls=systemcls)
# TODO: handle doc comments.x
assert capsys.readouterr().out == '<test>:18: Existing docstring at line 14 is overriden\n'
Loading