Merge pull request #6234 from asottile/remove_none_warning

Revert "A warning is now issued when assertions are made for `None`"

changelog/4639.feature.rst

@@ -0,0 +1,3 @@
+Revert "A warning is now issued when assertions are made for ``None``".
+The warning proved to be less useful than initially expected and had quite a
+few false positive cases.

src/_pytest/assertion/rewrite.py

@@ -799,13 +799,6 @@ def visit_Assert(self, assert_):
         self.push_format_context()
         # Rewrite assert into a bunch of statements.
         top_condition, explanation = self.visit(assert_.test)
-        # If in a test module, check if directly asserting None, in order to warn [Issue #3191]
-        if self.module_path is not None:
-            self.statements.append(
-                self.warn_about_none_ast(
-                    top_condition, module_path=self.module_path, lineno=assert_.lineno
-                )
-            )
 
         negation = ast.UnaryOp(ast.Not(), top_condition)
 
@@ -887,30 +880,6 @@ def visit_Assert(self, assert_):
             set_location(stmt, assert_.lineno, assert_.col_offset)
         return self.statements
 
-    def warn_about_none_ast(self, node, module_path, lineno):
-        """
-        Returns an AST issuing a warning if the value of node is `None`.
-        This is used to warn the user when asserting a function that asserts
-        internally already.
-        See issue #3191 for more details.
-        """
-        val_is_none = ast.Compare(node, [ast.Is()], [ast.NameConstant(None)])
-        send_warning = ast.parse(
-            """\
-from _pytest.warning_types import PytestAssertRewriteWarning
-from warnings import warn_explicit
-warn_explicit(
-    PytestAssertRewriteWarning('asserting the value None, please use "assert is None"'),
-    category=None,
-    filename={filename!r},
-    lineno={lineno},
-)
-            """.format(
-                filename=fspath(module_path), lineno=lineno
-            )
-        ).body
-        return ast.If(val_is_none, send_warning, [])
-
     def visit_Name(self, name):
         # Display the repr of the name if it's a local variable or
         # _should_repr_global_name() thinks it's acceptable.

testing/test_warnings.py

@@ -543,7 +543,7 @@ def assert_result_warns(result, msg):
 
     def test_tuple_warning(self, testdir):
         testdir.makepyfile(
-            """
+            """\
             def test_foo():
                 assert (1,2)
             """
@@ -553,48 +553,6 @@ def test_foo():
             result, "assertion is always true, perhaps remove parentheses?"
         )
 
-    @staticmethod
-    def create_file(testdir, return_none):
-        testdir.makepyfile(
-            """
-            def foo(return_none):
-                if return_none:
-                    return None
-                else:
-                    return False
-
-            def test_foo():
-                assert foo({return_none})
-            """.format(
-                return_none=return_none
-            )
-        )
-
-    def test_none_function_warns(self, testdir):
-        self.create_file(testdir, True)
-        result = testdir.runpytest()
-        self.assert_result_warns(
-            result, 'asserting the value None, please use "assert is None"'
-        )
-
-    def test_assert_is_none_no_warn(self, testdir):
-        testdir.makepyfile(
-            """
-            def foo():
-                return None
-
-            def test_foo():
-                assert foo() is None
-            """
-        )
-        result = testdir.runpytest()
-        result.stdout.fnmatch_lines(["*1 passed in*"])
-
-    def test_false_function_no_warn(self, testdir):
-        self.create_file(testdir, False)
-        result = testdir.runpytest()
-        result.stdout.fnmatch_lines(["*1 failed in*"])
-
 
 def test_warnings_checker_twice():
     """Issue #4617"""