Merge remote-tracking branch 'upstream/main' into documentationTen

.github/PULL_REQUEST_TEMPLATE.md

@@ -3,11 +3,18 @@ Thank you for submitting a PR to pylint!
 
 To ease the process of reviewing your PR, do make sure to complete the following boxes.
 
-- [ ] Write a good description on what the PR does.
-- [ ] Create a news fragment with `towncrier create <IssueNumber>.<type>` which will be
-  included in the changelog. `<type>` can be one of: breaking, user_action, feature,
-  new_check, removed_check, extension, false_positive, false_negative, bugfix, other, internal.
-  If necessary you can write details or offer examples on how the new change is supposed to work.
+- [ ] Document your change, if it is a non-trivial one.
+  - A maintainer might label the issue ``skip-news`` if the change does not need to be in the changelog.
+  - Otherwise, create a news fragment with ``towncrier create <IssueNumber>.<type>`` which will be
+    included in the changelog. ``<type>`` can be one of the types defined in `./towncrier.toml`.
+    If necessary you can write details or offer examples on how the new change is supposed to work.
+  - Generating the doc is done with ``tox -e docs``
+- [ ] Relate your change to an issue in the tracker if such an issue exists (Refs #1234, Closes #1234)
+- [ ] Write comprehensive commit messages and/or a good description of what the PR does.
+- [ ] Keep the change small, separate the consensual changes from the opinionated one.
+  Don't hesitate to open multiple PRs if the change requires it. If your review is so
+  big it requires to actually plan and allocate time to review, it's more likely
+  that it's going to go stale.
 - [ ] If you used multiple emails or multiple names when contributing, add your mails
       and preferred name in ``script/.contributors_aliases.json``
 -->

doc/data/messages/s/suppressed-message/bad.py

@@ -0,0 +1,9 @@
+### This is a contrived example, to show how suppressed-message works.
+### First we enable all messages
+# pylint: enable=all
+
+### Here we disable two messages so we get two warnings
+# pylint: disable=locally-disabled, useless-suppression # [suppressed-message, suppressed-message]
+
+### Here we disable a message, so we get a warning for suppressed-message again.
+"A"  # pylint: disable=pointless-statement # [suppressed-message, suppressed-message]

doc/data/messages/s/suppressed-message/details.rst

@@ -1 +1,4 @@
-You can help us make the doc better `by contributing <https://github.com/PyCQA/pylint/issues/5953>`_ !
+``suppressed-message`` is simply a way to see messages that would be raised
+without the disable in your codebase. It should not be activated most
+of the time. See also ``useless-suppression`` if you want to see the message
+that are disabled for no reasons.

doc/data/messages/s/suppressed-message/good.py

@@ -1 +1 @@
-# This is a placeholder for correct code for this message.
+"""Instead of a single string somewhere in the file, write a module docstring!"""

doc/development_guide/contributor_guide/contribute.rst

@@ -67,24 +67,32 @@ your patch gets accepted:
 
 .. keep this in sync with the description of PULL_REQUEST_TEMPLATE.md!
 
-- Create a news fragment with `towncrier create <IssueNumber>.<type>` which will be
-  included in the changelog. `<type>` can be one of: breaking, user_action, feature,
-  new_check, removed_check, extension, false_positive, false_negative, bugfix, other, internal.
-  If necessary you can write details or offer examples on how the new change is supposed to work.
+- Document your change, if it is a non-trivial one:
 
-- Document your change, if it is a non-trivial one.
+  * A maintainer might label the issue ``skip-news`` if the change does not need to be in the changelog.
+  * Otherwise, create a news fragment with ``towncrier create <IssueNumber>.<type>`` which will be
+    included in the changelog. ``<type>`` can be one of the types defined in `./towncrier.toml`.
+    If necessary you can write details or offer examples on how the new change is supposed to work.
+  * Generating the doc is done with ``tox -e docs``
 
-- If you used multiple emails or multiple names when contributing, add your mails
-  and preferred name in the ``script/.contributors_aliases.json`` file.
+- Send a pull request from GitHub (see `About pull requests`_ for more insight about this topic).
 
-- Write a comprehensive commit message
-
-- Relate your change to an issue in the tracker if such an issue exists (see
+- Write comprehensive commit messages and/or a good description of what the PR does.
+  Relate your change to an issue in the tracker if such an issue exists (see
   `Closing issues via commit messages`_ of the GitHub documentation for more
   information on this)
 
-- Send a pull request from GitHub (see `About pull requests`_ for more insight
-  about this topic)
+- Keep the change small, separate the consensual changes from the opinionated one.
+
+  * Don't hesitate to open multiple PRs if the change requires it. If your review is so
+    big it requires to actually plan and allocate time to review, it's more likely
+    that it's going to go stale.
+  * Maintainers might have multiple 5 to 10 minutes review windows per day, Say while waiting
+    for their teapot to boil, or for their partner to recover from their hilarious nerdy joke,
+    but only one full hour review time per week, if at all.
+
+- If you used multiple emails or multiple names when contributing, add your mails
+  and preferred name in the ``script/.contributors_aliases.json`` file.
 
 .. _`Closing issues via commit messages`: https://github.blog/2013-01-22-closing-issues-via-commit-messages/
 .. _`About pull requests`: https://support.github.com/features/pull-requests

doc/development_guide/contributor_guide/governance.rst

@@ -0,0 +1,110 @@
+============
+ Governance
+============
+
+How to become part of the project ?
+-----------------------------------
+
+How to become a contributor ?
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+- Follow the code of conduct
+- Search by yourself before asking for help
+- Open an issue
+- Investigate an issue and report your finding
+- Open a merge request directly if you feel it's a consensual change
+
+Reporting a bug is being a contributor already.
+
+How to become a triager ?
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+- Contribute for more than 3 releases consistently.
+- Do not be too opinionated, follow the code of conduct without requiring emotional
+  works from the maintainers. It does not mean that disagreements are impossible,
+  only that arguments should stay technical and reasonable so the conversation
+  is civil and productive.
+- Have a maintainer suggest that you become triager, without you asking
+- Get unanimous approval or neutral agreement from current maintainers.
+
+How to become a maintainer ?
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+- Contribute two big code merge requests over multiple releases (for example
+  one checker in 2.13 and the following bug after release and one complicated
+  bug fixes in 2.14). Otherwise contributing for more than 3 releases consistently
+  with great technical and interpersonal skills.
+- Take ownership of a part of the code that is not maintained well at the moment
+  or that you contributed personally (if we feel we can't merge something without
+  your review, you're going to be able to merge those yourself soon).
+- Triage for multiple months (close duplicate, clean up issues, answer questions...)
+- Have an admin suggest that you become maintainer, without you asking
+- Get unanimous approval or neutral agreement from current maintainers.
+
+
+How to become an admin ?
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+- Contribute for several hundreds of commits over a long period of time
+  with excellent interpersonal skills and code quality.
+- Maintain pylint for multiple years (code review, triaging and maintenance tasks).
+- At this point probably have another admin leave the project or
+  become inactive for years.
+- Have an admin suggest that you become an admin, without you asking.
+- Get unanimous approval or neutral agreement from current admins.
+
+
+How are decisions made ?
+------------------------
+
+Everyone is expected to follow the code of conduct. pylint is a do-ocracy / democracy.
+You're not allowed to behave poorly because you contributed a lot. But if
+you're not going to do the future maintenance work, your valid opinions might not be
+taken into account by those that will be affected by it.
+
+What are the fundamental tenets of pylint development?
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+General:
+
+- We favor correctness over performance, because pylint is not used primarily
+  for its performance. Performance is still important and needs to be taken into
+  account from the get go.
+
+- We then favor false negatives over false positives if correctness is
+  impossible to achieve.
+
+- We try to keep the configuration sane, but if there's a hard decision to take we
+  add an option so that pylint is multiple sizes fit all (after configuration)
+
+Where to add a new checker or message:
+
+- Error messages (things that will result in an error if run) should be builtin
+  checks, activated by default
+
+- Messages that are opinionated, even slightly, should be opt-in (added as :ref:`an extension<plugins>`)
+
+- We don't shy away from opinionated checks (like the while checker), but there's such a
+  thing as too opinionated, if something is too opinionated it should be an external
+  :ref:`pylint plugin<plugins>`.
+
+How are disagreements handled ?
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+When something is not consensual users, maintainers, and admins discuss until an
+agreement is reached.
+
+Depending on the difficulty of the discussion and the importance of a fast resolution,
+a decision can be taken:
+
+- Unanimously between discussion participants, contributors and maintainers (preferably)
+
+- By asking discussion participants for their opinions with an emoji survey in the
+  issue and then using the majority if no maintainers feel strongly about the issue.
+
+- By majority of admins if no admins feel strongly about the issue.
+
+- By asking all users for their opinions in a new issue that will be pinned for
+  multiple months before taking the decision if two admins feel strongly on an
+  opposite side of the issue. Once the result is obvious the majority decision
+  is not up for discussion anymore.

doc/development_guide/contributor_guide/index.rst

@@ -13,3 +13,4 @@ The contributor guide will help you if you want to contribute to pylint itself.
    tests/index
    profiling
    release
+   governance

doc/exts/pylint_messages.py

@@ -84,7 +84,7 @@ def _get_example_code(data_path: Path) -> str:
     )
 
     _check_placeholders(data_path, bad_code, details, related)
-    return "\n".join((good_code, bad_code, pylintrc, details, related)) + "\n"
+    return "\n".join((bad_code, good_code, pylintrc, details, related)) + "\n"
 
 
 def _get_pylintrc_code(data_path: Path) -> str:
@@ -307,7 +307,7 @@ def _generate_single_message_body(message: MessageData) -> str:
 {get_rst_title(f"{message.name} / {message.id}", "=")}
 **Message emitted:**
 
-{message.definition.msg}
+``{message.definition.msg}``
 
 **Description:**
 

doc/whatsnew/fragments/8280.false_negative

@@ -0,0 +1,3 @@
+Fix false negative for inconsistent-returns with while-loops.
+
+Closes #8280

doc/whatsnew/fragments/8329.other

@@ -0,0 +1,5 @@
+The governance model and the path to become a maintainer have been documented as
+part of our effort to guarantee that the software supply chain in which pylint is
+included is secure.
+
+Refs #8329

pylint/checkers/refactoring/refactoring_checker.py

@@ -20,6 +20,7 @@
 
 from pylint import checkers
 from pylint.checkers import utils
+from pylint.checkers.base.basic_error_checker import _loop_exits_early
 from pylint.checkers.utils import node_frame_class
 from pylint.interfaces import HIGH, INFERENCE, Confidence
 
@@ -1945,10 +1946,12 @@ def _is_node_return_ended(self, node: nodes.NodeNG) -> bool:
                     return True
             except astroid.InferenceError:
                 pass
-        # Avoid the check inside while loop as we don't know
-        # if they will be completed
         if isinstance(node, nodes.While):
-            return True
+            # A while-loop is considered return-ended if it has a
+            # truthy test and no break statements
+            return (node.test.bool_value() and not _loop_exits_early(node)) or any(
+                self._is_node_return_ended(child) for child in node.orelse
+            )
         if isinstance(node, nodes.Raise):
             return self._is_raise_node_return_ended(node)
         if isinstance(node, nodes.If):

pylint/checkers/stdlib.py

@@ -386,7 +386,7 @@ class StdlibChecker(DeprecatedMixin, BaseChecker):
             "By default, the first parameter is the group param, not the target param.",
         ),
         "W1507": (
-            "Using copy.copy(os.environ). Use os.environ.copy() instead. ",
+            "Using copy.copy(os.environ). Use os.environ.copy() instead.",
             "shallow-copy-environ",
             "os.environ is not a dict object but proxy object, so "
             "shallow copy has still effects on original object. "

tests/functional/i/inconsistent/inconsistent_returns.py

@@ -1,5 +1,5 @@
 #pylint: disable=missing-docstring, no-else-return, no-else-break, invalid-name, unused-variable, superfluous-parens, try-except-raise
-#pylint: disable=disallowed-name
+#pylint: disable=disallowed-name,too-few-public-methods,no-member,useless-else-on-loop
 """Testing inconsistent returns"""
 import math
 import sys
@@ -353,3 +353,52 @@ def bug_pylint_4019_wrong(x):  # [inconsistent-return-statements]
     if x == 1:
         return 42
     assert True
+
+
+# https://github.com/PyCQA/pylint/issues/8280
+class A:
+    def get_the_answer(self):  # [inconsistent-return-statements]
+        while self.is_running:
+            self.read_message()
+            if self.comunication_finished():
+                return "done"
+
+
+def bug_1772_break():  # [inconsistent-return-statements]
+    counter = 1
+    while True:
+        counter += 1
+        if counter == 100:
+            return 7
+        if counter is None:
+            break
+
+
+def while_break_in_for():
+    counter = 1
+    while True:
+        counter += 1
+        if counter == 100:
+            return 7
+        for i in range(10):
+            if i == 5:
+                break
+
+
+def while_break_in_while():
+    counter = 1
+    while True:
+        counter += 1
+        if counter == 100:
+            return 7
+        while True:
+            if counter == 5:
+                break
+
+
+def wait_for_apps_ready(event, main_thread):
+    while main_thread.is_alive():
+        if event.wait(timeout=0.1):
+            return True
+    else:
+        return False

tests/functional/i/inconsistent/inconsistent_returns.txt

@@ -15,3 +15,5 @@ inconsistent-return-statements:267:0:267:12:bug_3468:Either all return statement
 inconsistent-return-statements:277:0:277:20:bug_3468_variant:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
 inconsistent-return-statements:322:0:322:21:bug_pylint_3873_1:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
 inconsistent-return-statements:349:0:349:25:bug_pylint_4019_wrong:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
+inconsistent-return-statements:360:4:360:22:A.get_the_answer:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
+inconsistent-return-statements:367:0:367:18:bug_1772_break:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED

tests/functional/s/shallow_copy_environ.txt

@@ -1,2 +1,2 @@
-shallow-copy-environ:7:0:7:21::Using copy.copy(os.environ). Use os.environ.copy() instead. :UNDEFINED
-shallow-copy-environ:17:0:17:18::Using copy.copy(os.environ). Use os.environ.copy() instead. :UNDEFINED
+shallow-copy-environ:7:0:7:21::Using copy.copy(os.environ). Use os.environ.copy() instead.:UNDEFINED
+shallow-copy-environ:17:0:17:18::Using copy.copy(os.environ). Use os.environ.copy() instead.:UNDEFINED