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

Add support for more complicated documentation examples #8287

Merged
merged 6 commits into from
Feb 20, 2023
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
14 changes: 14 additions & 0 deletions doc/data/messages/d/duplicate-code/bad/apple.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class Apple:
def __init__(self):
self.remaining_bites = 3

def take_bite(self):
if self.remaining_bites > 0:
print("You take a bite of the apple.")
self.remaining_bites -= 1
else:
print("The apple is already eaten up!")

def eaten_by_animal(self, animal):
self.remaining_bites = 0
print("The apple has been eaten by an animal.")
16 changes: 16 additions & 0 deletions doc/data/messages/d/duplicate-code/bad/orange.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
class Orange: # [duplicate-code]
def __init__(self):
self.remaining_bites = 3

def take_bite(self):
if self.remaining_bites > 0:
print("You take a bite of the apple.")
self.remaining_bites -= 1
else:
print("The orange is already eaten up!")

def eaten_by_animal(self, animal):
if animal == "cat":
raise ValueError("A cat would never do that !")
self.remaining_bites = 0
print("The orange has been eaten by an animal.")
10 changes: 9 additions & 1 deletion doc/data/messages/d/duplicate-code/details.rst
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
You can help us make the doc better `by contributing <https://github.com/PyCQA/pylint/issues/5953>`_ !
If you need to make a change to the logic or functionality of the duplicated
code, you will need to identify all the places that need to be changed, which
can be time-consuming and error-prone. If there are multiple copies of the
same code, then you will also need to test each copy to ensure that the
functionality is correct. Duplicate code can be confusing for someone who is
trying to understand the logic and flow of the code if they come across multiple
identical or nearly identical blocks of code. The reader can then skim and
think something is identical when it actually isn't. This is particularly true
during review.
1 change: 0 additions & 1 deletion doc/data/messages/d/duplicate-code/good.py

This file was deleted.

5 changes: 5 additions & 0 deletions doc/data/messages/d/duplicate-code/good/apple.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from fruit import Fruit


class Apple(Fruit):
...
14 changes: 14 additions & 0 deletions doc/data/messages/d/duplicate-code/good/fruit.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class Fruit:
def __init__(self):
self.remaining_bites = 3

def take_bite(self):
if self.remaining_bites > 0:
print(f"You take a bite of the {self.__class__.__name__.lower()}.")
self.remaining_bites -= 1
else:
print(f"The {self.__class__.__name__.lower()} is already eaten up!")

def eaten_by_animal(self, animal):
self.remaining_bites = 0
print(f"The {self.__class__.__name__.lower()} has been eaten by an animal.")
8 changes: 8 additions & 0 deletions doc/data/messages/d/duplicate-code/good/orange.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from fruit import Fruit


class Orange(Fruit):
def eaten_by_animal(self, animal):
if animal == "cat":
raise ValueError("A cat would never do that !")
super().eaten_by_animal(animal)
137 changes: 85 additions & 52 deletions doc/exts/pylint_messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import os
from collections import defaultdict
from enum import Enum
from inspect import getmodule
from itertools import chain, groupby
from pathlib import Path
Expand Down Expand Up @@ -39,16 +40,18 @@ class MessageData(NamedTuple):
id: str
name: str
definition: MessageDefinition
good_code: str
bad_code: str
details: str
related_links: str
example_code: str
checker_module_name: str
checker_module_path: str
shared: bool = False
default_enabled: bool = True


class ExampleType(str, Enum):
GOOD = "good"
BAD = "bad"


MessagesDict = Dict[str, List[MessageData]]
OldMessagesDict = Dict[str, DefaultDict[Tuple[str, str], List[Tuple[str, str]]]]
"""DefaultDict is indexed by tuples of (old name symbol, old name id) and values are
Expand All @@ -62,39 +65,89 @@ def _register_all_checkers_and_extensions(linter: PyLinter) -> None:
initialize_extensions(linter)


def _get_message_data(data_path: Path) -> tuple[str, str, str, str]:
"""Get the message data from the specified path."""
good_py_path = data_path / "good.py"
bad_py_path = data_path / "bad.py"
details_rst_path = data_path / "details.rst"
related_rst_path = data_path / "related.rst"
def _get_example_code(data_path: Path) -> str:
"""Get the example code from the specified path."""
if not data_path.exists():
_create_placeholders(data_path, details_rst_path, good_py_path)
good_code = _get_titled_rst(
title="Correct code", text=_get_python_code_as_rst(good_py_path)
)
bad_code = _get_titled_rst(
title="Problematic code", text=_get_python_code_as_rst(bad_py_path)
)
raise AssertionError(
f"Documentation examples path {data_path} does not exist. "
"Please create it and add an example."
)

good_code = _get_demo_code_for(data_path, ExampleType.GOOD)
bad_code = _get_demo_code_for(data_path, ExampleType.BAD)
pylintrc = _get_pylintrc_code(data_path)
details = _get_titled_rst(
title="Additional details", text=_get_rst_as_str(details_rst_path)
title="Additional details", text=_get_rst_as_str(data_path / "details.rst")
)
related = _get_titled_rst(
title="Related links", text=_get_rst_as_str(related_rst_path)
title="Related links", text=_get_rst_as_str(data_path / "related.rst")
)
_check_placeholders(bad_code, details, good_py_path, related)
return good_code, bad_code, details, related

_check_placeholders(data_path, bad_code, details, related)
return "\n".join((good_code, bad_code, pylintrc, details, related)) + "\n"


def _get_pylintrc_code(data_path: Path) -> str:
if (data_path / "pylintrc").exists():
pylintrc = _get_titled_rst(
title="Configuration file", text=_get_ini_as_rst(data_path / "pylintrc")
)
else:
pylintrc = ""
return pylintrc


def _get_demo_code_for(data_path: Path, example_type: ExampleType) -> str:
"""Get code examples while handling multi-file code templates."""
single_file_path = data_path / f"{example_type.value}.py"
multiple_code_path = data_path / f"{example_type.value}"

if single_file_path.exists() and multiple_code_path.exists():
raise ValueError(
f"You cannot have a single file '{example_type.value}.py' and multiple files "
f"example '{multiple_code_path}' existing at the same time."
)

title = "Problematic code" if example_type is ExampleType.BAD else "Correct code"
if single_file_path.exists():
return _get_titled_rst(
title=title, text=_get_python_code_as_rst(single_file_path)
)

if multiple_code_path.exists():
files: list[str] = []
# Sort so the order of the files makes sense
for file_as_str in sorted([str(p) for p in multiple_code_path.iterdir()]):
file = Path(file_as_str)
if file.suffix == ".py":
files.append(
f"""\
``{file.name}``:

.. literalinclude:: /{file.relative_to(Path.cwd())}
:language: python

"""
)
return _get_titled_rst(title=title, text="\n".join(files))

return ""


def _check_placeholders(
bad_code: str, details: str, good_py_path: Path, related: str
data_path: Path, bad_code: str, details: str, related: str
) -> None:
# Check if the placeholder file can even be presented by checking if its path exists
good_path = data_path / "good.py"
if not good_path.exists():
return

if bad_code or related:
placeholder_details = "help us make the doc better" in details
with open(good_py_path) as f:
with open(good_path, encoding="utf-8") as f:
placeholder_good = "placeholder" in f.read()
assert_msg = (
f"Please remove placeholders in '{good_py_path.parent}' "
f"Please remove placeholders in '{data_path}' "
f"as you started completing the documentation"
)
assert not placeholder_good and not placeholder_details, assert_msg
Expand Down Expand Up @@ -127,22 +180,11 @@ def _get_python_code_as_rst(code_path: Path) -> str:
"""


def _create_placeholders(
data_path: Path, details_rst_path: Path, good_py_path: Path
) -> None:
data_path.mkdir(parents=True)
with open(good_py_path, "w", encoding="utf-8") as file:
file.write(
"""\
# This is a placeholder for correct code for this message.
"""
)
with open(details_rst_path, "w", encoding="utf-8") as file:
file.write(
"""\
You can help us make the doc better `by contributing <https://github.com/PyCQA/pylint/issues/5953>`_ !
def _get_ini_as_rst(code_path: Path) -> str:
return f"""\
.. literalinclude:: /{code_path.relative_to(Path.cwd())}
:language: ini
"""
)


def _get_all_messages(
Expand Down Expand Up @@ -175,9 +217,7 @@ def _get_all_messages(
)

for checker, message in checker_message_mapping:
good_code, bad_code, details, related = _get_message_data(
_get_message_data_path(message)
)
example_code = _get_example_code(_get_message_data_path(message))

checker_module = getmodule(checker)

Expand All @@ -190,10 +230,7 @@ def _get_all_messages(
message.msgid,
message.symbol,
message,
good_code,
bad_code,
details,
related,
example_code,
checker_module.__name__,
checker_module.__file__,
message.shared,
Expand Down Expand Up @@ -283,12 +320,8 @@ def _generate_single_message_body(message: MessageData) -> str:

"""

body += f"""
{message.bad_code}
{message.good_code}
{message.details}
{message.related_links}
"""
body += "\n" + message.example_code + "\n"

if message.checker_module_name.startswith("pylint.extensions."):
body += f"""
.. note::
Expand Down
60 changes: 41 additions & 19 deletions doc/test_messages_documentation.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,8 @@ def get_functional_test_files_from_directory(input_dir: Path) -> list[tuple[str,
f"directory: it does not start with '{subdirectory.name}'"
)
assert message_dir.name.startswith(subdirectory.name), assert_msg
if (message_dir / "good.py").exists():
suite.append(
(message_dir.stem, message_dir / "good.py"),
)
if (message_dir / "bad.py").exists():
suite.append(
(message_dir.stem, message_dir / "bad.py"),
)
_add_code_example_to_suite(message_dir, suite, "good")
_add_code_example_to_suite(message_dir, suite, "bad")
if (message_dir / "related.rst").exists():
with open(message_dir / "related.rst", encoding="utf-8") as file:
text = file.read()
Expand All @@ -65,6 +59,28 @@ def get_functional_test_files_from_directory(input_dir: Path) -> list[tuple[str,
return suite


def _add_code_example_to_suite(
message_dir: Path, suite: list[tuple[str, Path]], example_type: str
) -> None:
"""Code example files can either consist of a single file or a directory."""
file = f"{example_type}.py"
directory = f"{example_type}"
if (message_dir / file).exists():
suite.append(
(message_dir.stem, message_dir / file),
)
elif (message_dir / directory).is_dir():
dir_to_add = message_dir / directory
len_to_add = len(list(dir_to_add.iterdir()))
assert len_to_add > 1, (
f"A directory of {example_type} files needs at least two files, "
f"but only found one in {dir_to_add}."
)
suite.append(
(message_dir.stem, dir_to_add),
)


TESTS_DIR = Path(__file__).parent.resolve() / "data" / "messages"
TESTS = get_functional_test_files_from_directory(TESTS_DIR)
TESTS_NAMES = [f"{t[0]}-{t[1].stem}" for t in TESTS]
Expand Down Expand Up @@ -103,11 +119,11 @@ def __init__(self, test_file: tuple[str, Path]) -> None:
def runTest(self) -> None:
self._runTest()

def is_good_test_file(self) -> bool:
return self._test_file[1].name == "good.py"
def is_good_test(self) -> bool:
return self._test_file[1].stem == "good"

def is_bad_test_file(self) -> bool:
return self._test_file[1].name == "bad.py"
def is_bad_test(self) -> bool:
return self._test_file[1].stem == "bad"

@staticmethod
def get_expected_messages(stream: TextIO) -> MessageCounter:
Expand All @@ -131,9 +147,15 @@ def get_expected_messages(stream: TextIO) -> MessageCounter:
return messages

def _get_expected(self) -> MessageCounter:
"""Get the expected messages for a file."""
with open(self._test_file[1], encoding="utf8") as f:
expected_msgs = self.get_expected_messages(f)
"""Get the expected messages for a file or directory."""
expected_msgs: MessageCounter = Counter()
if self._test_file[1].is_dir():
for test_file in self._test_file[1].iterdir():
with open(test_file, encoding="utf8") as f:
expected_msgs += self.get_expected_messages(f)
else:
with open(self._test_file[1], encoding="utf8") as f:
expected_msgs += self.get_expected_messages(f)
return expected_msgs

def _get_actual(self) -> MessageCounter:
Expand All @@ -147,15 +169,15 @@ def _get_actual(self) -> MessageCounter:

def _runTest(self) -> None:
"""Run the test and assert message differences."""
self._linter.check([str(self._test_file[1]), "--rcfile="])
self._linter.check([str(self._test_file[1])])
expected_messages = self._get_expected()
actual_messages = self._get_actual()
if self.is_good_test_file():
if self.is_good_test():
assert actual_messages.total() == 0, self.assert_message_good(
actual_messages
)
if self.is_bad_test_file():
msg = "There should be at least one warning raised for 'bad.py'"
if self.is_bad_test():
msg = "There should be at least one warning raised for 'bad' examples."
assert actual_messages.total() > 0, msg
assert expected_messages == actual_messages

Expand Down
3 changes: 0 additions & 3 deletions pylint/testutils/reporter_for_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ def _display(self, layout: Section) -> None:


class FunctionalTestReporter(BaseReporter):
def on_set_current_module(self, module: str, filepath: str | None) -> None:
self.messages = []

Comment on lines -75 to -77
Copy link
Member

Choose a reason for hiding this comment

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

Nice !

def display_reports(self, layout: Section) -> None:
"""Ignore layouts and don't call self._display()."""

Expand Down
3 changes: 0 additions & 3 deletions tests/functional/e/empty_docstring.rc

This file was deleted.

Loading