Unassigned instantiated classes should trigger a pointless-statement or a new message to be determined

[vapier]

Current status

pylint will issue pointless-statement when a class (e.g. Exception) is used. this is great:

$ cat test.py
Exception
$ pylint3 test.py
test.py:1:0: W0104: Statement seems to have no effect (pointless-statement)

but if the exception is instantiated, it's not shown:

$ cat test.py
Exception()
$ pylint3 test.py
<nothing>

Proposal

while instantiating random classes can have side-effects, exceptions rarely (if ever) have such behavior. it's so uncommon that, imo, it should trigger a warning by default. we ran into issues where people forgot to raise the exception (because the naming made it sound like it would throw an error), so code that was supposed to fail ended up not.

so how about:

$ cat test.py
Exception()
$ pylint3 test.py
test.py:1:0: W0104: Instantiated object seems to have no effect (pointless-exception-object)

feel free to bikeshed the exact naming/phrasing.

Version info

$ pylint3 --version
pylint3 2.2.2
astroid 2.1.0
Python 3.6.8 (default, Jan  3 2019, 03:42:36) 
[GCC 8.2.0]

[vapier]

maybe something like:

    def visit_expr(self, node):
        if not isinstance(node.value, astroid.Call):
            return
        func = node.value.func
        try:
            cls = next(func.infer())
        except astroid.InferenceError:
            return
        if not isinstance(cls, astroid.ClassDef):
            return
        if any(x for x in cls.ancestors() if x.name == 'BaseException'):
            self.add_message('...', node=node, line=node.fromlineno)

[PCManticore]

This sounds good to me, but I think we should do it for the general case not just for exceptions. Instantiating a class and not doing anything with it sounds like a smelling to me (note that might have a purpose, e.g. testing that the instantiation raises an exception)

[vapier]

while i agree that it's a bit of an anti-pattern, i've seen it more than once e.g. instantiating a "run" or "main" object just to call the main method on it to transfer full control over. so that seemed more like taking a principle on style vs my more limited proposal here which seems to fall much more in the "it's a mistake/bug" bucket.

not sure if pylint has a preference for creating more/separate lint settings so people can differentiate between the two. it's unfortunate when a single warning class is disabled in a project because some of the issues it flags are "style" based, so the users miss out of on the "it's a bug" usage.

i think your suggestion could be accomplished right now by deleting a check in the existing code. untested diff:

--- a/pylint/checkers/base.py
+++ b/pylint/checkers/base.py
@@ -1125,7 +1125,6 @@ class BasicChecker(_BasicChecker):
             return

         # Ignore if this is :
-        # * a direct function call
         # * the unique child of a try/except body
         # * a yield statement
         # * an ellipsis (which can be used on Python 3 instead of pass)
@@ -1133,7 +1132,7 @@ class BasicChecker(_BasicChecker):
         # side effects), else pointless-statement
         if (
             isinstance(
-                expr, (astroid.Yield, astroid.Await, astroid.Ellipsis, astroid.Call)
+                expr, (astroid.Yield, astroid.Await, astroid.Ellipsis)
             )
             or (
                 isinstance(node.parent, astroid.TryExcept)

[PCManticore]

@vapier From my understanding, this new check will handle the following case:

Class()

while Class().run() and c = Class(); c.run() will not be caught.

The former looks more of an application bug than a code smell, as that instantiation might miss an additional method call or a variable assignment.

[Pierre-Sassoulas]

This is still relevant as

from pathlib import Path

Exception()
Path()

does not raise any message.

[jayaddison]

This issue is now covered by a check in pylint v2.16:

$ cat test.py 
Exception()
$ pylint test.py
************* Module test
test.py:1:0: C0114: Missing module docstring (missing-module-docstring)
test.py:1:0: W0133: Exception statement has no effect (pointless-exception-statement)

------------------------------------------------------------------
Your code has been rated at 0.00/10 (previous run: 0.00/10, +0.00)

[Pierre-Sassoulas]

Thank you for the help triaging @jayaddison :) Indeed one of the highlights of 2.16 for me !