Merge pull request #1001 from dodona-edu/chore/update-pylint

Update pylint to v4

Author: chvp

src/backend/workers/python/build_package.py

@@ -45,4 +45,4 @@ def check_tar(tarname, out_dir="."):
 
 
 if __name__ == "__main__":
-    create_package("python_package", "python-runner friendly_traceback pylint<3.0.0 tomli typing-extensions json-tracer>=0.7.0", extra_deps="papyros")
+    create_package("python_package", "python-runner friendly_traceback pylint>=4,<5 tomli typing-extensions json-tracer>=0.7.0", extra_deps="papyros")

src/backend/workers/python/papyros/linting.py

@@ -7,20 +7,6 @@
 from pylint.lint import Run
 from pylint.reporters.text import TextReporter
 
-# Workaround for Pyodide + astroid: pylint hangs indefinitely when astroid
-# tries to recursively parse imported modules' ASTs (e.g. re, pandas).
-# Since we run in a single-threaded WebAssembly environment, this causes an
-# infinite hang. We short-circuit ALL module resolution to return synthetic
-# empty modules. This preserves core linting (syntax errors, undefined
-# variables, unused imports, style checks, custom checkers) while only
-# losing type-inference-based checks on imported symbols.
-from astroid.manager import AstroidManager as _AstroidManager
-from astroid.builder import AstroidBuilder as _AstroidBuilder
-
-def _patched_ast_from_module_name(self, modname, context_file=None, use_cache=True):
-    return _AstroidBuilder(self).string_build("", modname=modname)
-
-_AstroidManager.ast_from_module_name = _patched_ast_from_module_name
 
 PYLINT_RC_FILE = os.path.abspath("/tmp/papyros/pylint_config.rc")
 PYLINT_PLUGINS = "pylint_ast_checker"

src/backend/workers/python/papyros/papyros.py

@@ -305,21 +305,8 @@ def serialize_traceback(self, exc):
 
     def lint(self, code):
         with self._without_file_tracking():
-            # PyLint runs into an issue when trying to import its dependencies
-            # Temporarily overriding os.devnull solves this issue
-            TEMP_DEV_NULL = "/home/pyodide/__papyros_dev_null"
-            with open(TEMP_DEV_NULL, "w") as f:
-                pass
-            orig_dev_null = os.devnull
-            os.devnull = TEMP_DEV_NULL
-
             self.set_source_code(code)
             from .linting import lint
-            os.devnull = orig_dev_null
-            try:
-                os.remove(TEMP_DEV_NULL)
-            except OSError:
-                pass
             return lint(code)
 
     def has_doctests(self, code):

src/backend/workers/python/papyros/pylint_ast_checker.py

@@ -1,13 +1,10 @@
 # source: https://github.com/dodona-edu/judge-pythia/blob/e41f6e943830f5f9b5aebe689140ec7ec53383c9/pylint_ast_checker.py
-import astroid
+from astroid import nodes as astroid_nodes
 from pylint.checkers import BaseChecker
-from pylint.checkers.utils import check_messages
-from pylint.interfaces import IAstroidChecker
+from pylint.checkers.utils import only_required_for_messages
 
 class NoOpChecker(BaseChecker):
 
-    __implements__ = IAstroidChecker
-
     name = 'no_op_checker'
 
     msgs = {
@@ -43,59 +40,57 @@ class NoOpChecker(BaseChecker):
         )
     }
 
-    @check_messages('no-op-pass')
+    @only_required_for_messages('no-op-pass')
     def visit_pass(self, node):
-        
-        if isinstance(node.parent, astroid.If):
+
+        if isinstance(node.parent, astroid_nodes.If):
             if len(node.parent.orelse) > 0 and node.parent.orelse[0] is node:
                 self.add_message('no-op-pass', node=node)
 
-    @check_messages('no-op-if-true')
-    def visit_if(self,node):
-        
+    @only_required_for_messages('no-op-if-true')
+    def visit_if(self, node):
+
         if (
-            isinstance(node.test, astroid.Compare) and 
-            any(operation =='==' for operation,_ in node.test.ops) and
-            any(isinstance(operand, astroid.Const) and 
+            isinstance(node.test, astroid_nodes.Compare) and
+            any(operation == '==' for operation,_ in node.test.ops) and
+            any(isinstance(operand, astroid_nodes.Const) and
             operand.value is True for operand in node.test.get_children())
         ):
             self.add_message('no-op-if-true', node=node)
 
-    @check_messages('no-op-increment-zero', 'no-op-increment-empty', 'no-op-multiply-one')
-    def visit_augassign(self,node):
-        
-        if node.op == '+=' and isinstance(node.value, astroid.Const):
+    @only_required_for_messages('no-op-increment-zero', 'no-op-increment-empty', 'no-op-multiply-one')
+    def visit_augassign(self, node):
+
+        if node.op == '+=' and isinstance(node.value, astroid_nodes.Const):
             if node.value.value == 0:
                 self.add_message('no-op-increment-zero', node=node)
             elif node.value.value == '':
-                self.add_message('no-op-increment-empty',node=node)
+                self.add_message('no-op-increment-empty', node=node)
         elif (
-            node.op =='*=' and 
-            isinstance(node.value, astroid.Const) and 
+            node.op == '*=' and
+            isinstance(node.value, astroid_nodes.Const) and
             node.value.value == 1
         ):
             self.add_message('no-op-multiply-one', node=node)
 
-    @check_messages('no-op-useless-assign')
-    def visit_assign(self,node):
-        
+    @only_required_for_messages('no-op-useless-assign')
+    def visit_assign(self, node):
+
         # only keep variables, e.g. [x, y, 2] -> [x, y]
         child_names = [
-            child.name for child in node.get_children() 
+            child.name for child in node.get_children()
             if (
-                isinstance(child,astroid.Name) or 
-                isinstance(child, astroid.AssignName)
+                isinstance(child, astroid_nodes.Name) or
+                isinstance(child, astroid_nodes.AssignName)
             )
         ]
-        
+
         if len(child_names) > len(set(child_names)):
             # at least two names were equal (e.g. ['x', 'y', 'x')
             self.add_message('no-op-useless-assign', node=node)
 
 class RewriteChecker(BaseChecker):
 
-    __implements__ = IAstroidChecker
-    
     name = 'rewrite_checker'
 
     msgs = {
@@ -124,98 +119,101 @@ class RewriteChecker(BaseChecker):
         'is not': 'is'
     }
 
-    @check_messages('rewrite-if-pass')
-    def visit_if(self,node):
-        
+    @only_required_for_messages('rewrite-if-pass')
+    def visit_if(self, node):
+
         # check whether the body of the if clause starts with the pass statement
         # and there is an else clause
-        if isinstance(node.body[0], astroid.Pass) and len(node.orelse) > 0:
-            
-            # swap body of the else clause in the if clause and remove the else 
+        if isinstance(node.body[0], astroid_nodes.Pass) and len(node.orelse) > 0:
+
+            # swap body of the else clause in the if clause and remove the else
             # clause
             node.body = node.orelse
             node.orelse = []
-            
+
             # invert the test
             # TODO: could be improved for recursive statements, etc.
             if (
-                isinstance(node.test, astroid.Compare) and 
+                isinstance(node.test, astroid_nodes.Compare) and
                 len(node.test.ops) == 1
             ):
                 node.test.ops[0] = (
                     self.inverse_operators[node.test.ops[0][0]],
                     node.test.ops[0][1]
                 )
             elif (
-                isinstance(node.test, astroid.UnaryOp) and 
+                isinstance(node.test, astroid_nodes.UnaryOp) and
                 node.test.op == 'not'
-            ): 
+            ):
                 # not <x> becomes <x>
                 node.test = node.test.operand
             else:
-                old_test, node.test = node.test, astroid.UnaryOp()
-                node.test.op, node.test.operand = 'not', old_test
-                
+                old_test = node.test
+                node.test = astroid_nodes.UnaryOp(
+                    op='not', lineno=node.lineno, col_offset=node.col_offset,
+                    parent=node, end_lineno=node.end_lineno, end_col_offset=node.end_col_offset,
+                )
+                node.test.operand = old_test
+
             # add message with corrected node as argument
             self.add_message(
-                'rewrite-if-pass', 
+                'rewrite-if-pass',
                 node=node,
                 args=node.as_string()
             )
 
-    @check_messages('rewrite-assign')
-    def visit_assign(self,node):
-        
+    @only_required_for_messages('rewrite-assign')
+    def visit_assign(self, node):
+
         # TODO: current implementation does not rewrite assignment statements
         #       having multiple targets or recursive binary operations
         #       (eg. x = x + 5 + 6)
         if len(node.targets) == 1:
-            
+
             target = node.targets[0]
-            
-            if isinstance(node.value, astroid.BinOp):
-                
+
+            if isinstance(node.value, astroid_nodes.BinOp):
+
                 rewrite = False
-                
+
                 # e.g. x = x + 4
                 if (
-                    isinstance(node.value.left, astroid.Name) and 
-                    isinstance(target, astroid.AssignName) and
+                    isinstance(node.value.left, astroid_nodes.Name) and
+                    isinstance(target, astroid_nodes.AssignName) and
                     node.value.left.name == target.name
                 ):
                     rewrite = True
 
                 # check for binary operations of the form x["y"] = x["y"] + 5
                 elif (
-                    isinstance(node.value.left, astroid.Subscript) and 
-                    isinstance(target, astroid.Subscript) and
-                    isinstance(node.value.left.value, astroid.Name) and 
-                    isinstance(target.value, astroid.Name) and
-                    node.value.left.value.name == target.value.name and
-                    isinstance(node.value.left.slice, astroid.Index) and 
-                    isinstance(target.slice, astroid.Index)
+                    isinstance(node.value.left, astroid_nodes.Subscript) and
+                    isinstance(target, astroid_nodes.Subscript) and
+                    isinstance(node.value.left.value, astroid_nodes.Name) and
+                    isinstance(target.value, astroid_nodes.Name) and
+                    node.value.left.value.name == target.value.name
                 ):
-                        # e.g. x[y] = x[y] + 1 (where y is a variable)
-                        if (
-                            isinstance(node.value.left.slice.value, astroid.Name) and 
-                            isinstance(target.slice.value, astroid.Name) and 
-                            node.value.left.slice.value.name == target.slice.value.name
-                        ):
-                            rewrite = True
-                        # e.g. x[0] = x[0] + 1 or x["y"] = x["y"] + 1
-                        elif (
-                            isinstance(node.value.left.slice.value, astroid.Const) and 
-                            isinstance(target.slice.value, astroid.Const) and 
-                            node.value.left.slice.value.value == target.slice.value.value
-                        ):
-                            rewrite = True
+                    # e.g. x[y] = x[y] + 1 (where y is a variable)
+                    if (
+                        isinstance(node.value.left.slice, astroid_nodes.Name) and
+                        isinstance(target.slice, astroid_nodes.Name) and
+                        node.value.left.slice.name == target.slice.name
+                    ):
+                        rewrite = True
+                    # e.g. x[0] = x[0] + 1 or x["y"] = x["y"] + 1
+                    elif (
+                        isinstance(node.value.left.slice, astroid_nodes.Const) and
+                        isinstance(target.slice, astroid_nodes.Const) and
+                        node.value.left.slice.value == target.slice.value
+                    ):
+                        rewrite = True
 
                 if rewrite:
-                    
-                    newnode = astroid.AugAssign()
+
+                    newnode = astroid_nodes.AugAssign(
+                        op=node.value.op + '=', lineno=node.lineno, col_offset=node.col_offset,
+                        parent=node.parent, end_lineno=node.end_lineno, end_col_offset=node.end_col_offset,
+                    )
                     newnode.target = target
-                    # node.value.op is of the form '+', '-' , '/' , '*'
-                    newnode.op = node.value.op + '=' 
                     newnode.value = node.value.right
                     self.add_message(
                         'rewrite-assign',
@@ -224,11 +222,9 @@ def visit_assign(self,node):
                     )
 
 class UnnecessaryConversionChecker(BaseChecker):
-    
-    __implements__ = IAstroidChecker
-    
+
     name = 'un_conv_checker'
-    
+
     msgs = {
         'C0009': (
             'Useless call to int(): no type conversion required.',
@@ -247,28 +243,28 @@ class UnnecessaryConversionChecker(BaseChecker):
         )
     }
 
-    @check_messages('un-conv-int', 'un-conv-float', 'un-conv-str')
-    def visit_callfunc(self, node):
-        
-        #check if 'regular' function:
-        if  hasattr(node.func, 'name'):
+    @only_required_for_messages('un-conv-int', 'un-conv-float', 'un-conv-str')
+    def visit_call(self, node):
+
+        # check if 'regular' function:
+        if hasattr(node.func, 'name'):
 
             # case: str(input(...))
             if (
                 # check if parent is str()
                 node.func.name == 'input' and
-                isinstance(node.parent, astroid.Call) and
+                isinstance(node.parent, astroid_nodes.Call) and
                 hasattr(node.parent.func, 'name') and
                 node.parent.func.name == 'str' and
                 # limited to a single argument
                 len(node.parent.args) == 1
             ):
                 self.add_message('un-conv-str', node=node.parent)
-            
+
             # check if a single constant argument is passed
             elif (
                 len(node.args) == 1 and
-                isinstance(node.args[0], astroid.Const)
+                isinstance(node.args[0], astroid_nodes.Const)
             ):
                 # case: int(<builtin.int>)
                 if (
@@ -290,11 +286,11 @@ def visit_callfunc(self, node):
                     self.add_message('un-conv-str', node=node)
 
 def register(linter):
-    
+
     """
     Required method to auto register custom checkers to pylint.
     """
-    
+
     linter.register_checker(NoOpChecker(linter))
     linter.register_checker(RewriteChecker(linter))
     linter.register_checker(UnnecessaryConversionChecker(linter))

src/backend/workers/python/papyros/pylint_config.rc

@@ -43,5 +43,5 @@ const-rgx=[_A-Za-z0-9]{1,30}$
 # I0011 Warning locally suppressed using disable-msg
 # I0012 Warning locally suppressed using disable-msg
 # old version: disable=I0011,I0012,W0704,W0142,W0212,W0232,W0702,R0201,W0614,R0914,R0912,R0915,R0913,R0904,R0801,C0303,C0111,C0304,R0903,W0141,W0621,C0301,W0631,R0911,C1001
-disable=W0311,W0621,W0622,R0902,R0903,C0111,C0301,C0303,C0304,C0413,I0011,E0401,E0611,E1101
+disable=W0311,W0621,W0622,R0902,R0903,C0114,C0115,C0116,C0301,C0303,C0304,C0413,I0011
 evaluation=max(10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10), 0)