Skip to content
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
2 changes: 1 addition & 1 deletion src/backend/workers/python/build_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
14 changes: 0 additions & 14 deletions src/backend/workers/python/papyros/linting.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 0 additions & 13 deletions src/backend/workers/python/papyros/papyros.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
chvp marked this conversation as resolved.

def has_doctests(self, code):
Expand Down
182 changes: 89 additions & 93 deletions src/backend/workers/python/papyros/pylint_ast_checker.py
Original file line number Diff line number Diff line change
@@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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',
Expand All @@ -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.',
Expand All @@ -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 (
Expand All @@ -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))
2 changes: 1 addition & 1 deletion src/backend/workers/python/papyros/pylint_config.rc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Loading