-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Condition Logic Checker #1114
Merged
david-yz-liu
merged 25 commits into
pyta-uoft:master
from
Raine-Yang-UofT:redundant-or-impossible-condition-checker
Dec 19, 2024
Merged
Condition Logic Checker #1114
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
e30f7fa
feat: implement basic checker feature
Raine-Yang-UofT 0f173f6
feat: implement impossible-condition-checker
Raine-Yang-UofT a9b24f6
fix: register checker to linter & add documentation
Raine-Yang-UofT 7bc4b7c
fix: fix typo in documentation
Raine-Yang-UofT a43c5bf
fix: fix type annotation for compatibility with older Python version
Raine-Yang-UofT b53c450
ci: remove z3 option to rerun CI tests
Raine-Yang-UofT 613e81a
fix: add node z3_constraint only if constraint is not none
Raine-Yang-UofT 65be1ed
tests: add docstrings in example test cases
Raine-Yang-UofT 4be4558
fix: fix failing unit tests
Raine-Yang-UofT 6593e85
refactor: revert changes
Raine-Yang-UofT 1d73c95
docs: update examples
Raine-Yang-UofT 10076c9
refactor: rename checker
Raine-Yang-UofT fbca50e
refactor: rename checker and remove outdated comments
Raine-Yang-UofT 54ad929
refactor: revert changes to graph.py
Raine-Yang-UofT 5ab960b
refactor: remove redundant helper function
Raine-Yang-UofT 76aeafc
docs: refine examples
Raine-Yang-UofT ec386e9
fix: revert file path changes to test_exampes
Raine-Yang-UofT 68f736d
Merge branch 'master' of https://github.com/pyta-uoft/pyta into redun…
Raine-Yang-UofT 78d4d2d
docs: refine documentation
Raine-Yang-UofT 950e18a
refactor: use Z3Environment to parse node condition
Raine-Yang-UofT ce10717
fix: fix failing test when multiple feasible branches are present
Raine-Yang-UofT a33367e
fix: fix condition logic error not ignored during reassignment
Raine-Yang-UofT e7a102c
fix: resolve merge conflicts
Raine-Yang-UofT 96aa91d
refactor: refine comments and documentation
Raine-Yang-UofT 1e0a29e
fix: fix false error reporting when condition is None
Raine-Yang-UofT File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
def return_large_number(x: int) -> int: | ||
"""Return number x only if it's greater than 1000 | ||
|
||
Preconditions: | ||
- x > 1000 | ||
""" | ||
# the if condition is already checked by function precondition | ||
if x > 1000: # Error on this line | ||
return x | ||
|
||
|
||
def nested_condition(x: int) -> int: | ||
if x > 10: | ||
# the condition `x > 5` is already guaranteed by `x > 10` | ||
if x > 5: # Error on this line | ||
return x | ||
return 0 | ||
|
||
|
||
def redundant_condition(x: bool) -> None: | ||
# the if condition is always true | ||
if x or not x: # Error on this line | ||
print("redundant") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
def print_none_negative_number(x: int) -> None: | ||
"""Print number x only if it's greater or equal to 0 | ||
|
||
Preconditions: | ||
- x >= 0 | ||
""" | ||
# the if condition is impossible given the function precondition | ||
if x < 0: # Error on this line | ||
raise Exception("x is smaller than zero") | ||
print(x) | ||
|
||
|
||
def impossible_condition(x: bool) -> None: | ||
# the if condition is always false | ||
if x and not x: # Error on this line | ||
print("impossible") | ||
|
||
|
||
def display_number(x: int) -> str: | ||
"""Display numbers 0 to 2 in word""" | ||
if x == 0: | ||
"zero" | ||
elif x == 1: | ||
return "one" | ||
elif x == 2: | ||
return "two" | ||
elif x == 2: # Error on this line | ||
return "two again" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
""" | ||
Check for redundant/impossible If or While conditions in functions based on z3 constraints | ||
""" | ||
|
||
from __future__ import annotations | ||
|
||
from typing import Any, Union | ||
|
||
try: | ||
import z3 | ||
|
||
from ..cfg.graph import Z3Environment | ||
|
||
z3_dependency_available = True | ||
except ImportError: | ||
z3 = Any | ||
Z3Environment = Any | ||
z3_dependency_available = False | ||
|
||
from astroid import nodes | ||
from pylint.checkers import BaseChecker | ||
from pylint.checkers.utils import only_required_for_messages | ||
from pylint.lint import PyLinter | ||
|
||
|
||
class ConditionLogicChecker(BaseChecker): | ||
name = "redundant-condition" | ||
msgs = { | ||
"R9900": ( | ||
"""This condition will always evaluate to True.""", | ||
"redundant-condition", | ||
"Used when an If or While statement is always True. Requires the z3 configuration option to be True.", | ||
), | ||
"R9901": ( | ||
"""This condition will always evaluate to False.""", | ||
"impossible-condition", | ||
"Used when an If or While statement is always False. Requires the z3 configuration option to be True.", | ||
), | ||
} | ||
options = ( | ||
( | ||
"z3", | ||
{ | ||
"default": False, | ||
"type": "yn", | ||
"metavar": "<y or n>", | ||
"help": "Use Z3 to perform logical feasibility analysis in program control flow.", | ||
}, | ||
), | ||
) | ||
|
||
@only_required_for_messages("redundant-condition", "impossible-condition") | ||
def visit_if(self, node: nodes.If) -> None: | ||
"""Visit if statement""" | ||
self._check_condition(node) | ||
|
||
@only_required_for_messages("redundant-condition", "impossible-condition") | ||
def visit_while(self, node: nodes.While) -> None: | ||
"""Visit while statement""" | ||
self._check_condition(node) | ||
|
||
def _check_condition(self, node: Union[nodes.If, nodes.While]) -> None: | ||
"""Check whether a condition in an `if` or `while` statement is redundant | ||
or impossible based on the feasible execution paths. | ||
|
||
- A condition is redundant if for every feasible execution path | ||
leading to the node, the condition must be True due to precedent constraints. | ||
- A condition is impossible if for every feasible execution path | ||
leading to the node, the condition must be False due to precedent constraints. | ||
""" | ||
if ( | ||
not hasattr(node, "cfg_block") | ||
or not z3_dependency_available | ||
or not self.linter.config.z3 | ||
): | ||
return | ||
|
||
node_block = node.cfg_block | ||
|
||
# create node condition z3 constraint | ||
condition_node = node.test | ||
env = Z3Environment(node.frame().cfg.z3_vars, []) | ||
z3_condition = env.parse_constraint(condition_node) | ||
|
||
if z3_condition is None: | ||
return | ||
|
||
if all( | ||
self._check_unsat(z3.And(*constraints), z3.Not(z3_condition)) | ||
for edge in (pred for pred in node_block.predecessors if pred.is_feasible) | ||
for constraints in edge.z3_constraints.values() | ||
): | ||
self.add_message("redundant-condition", node=node.test) | ||
|
||
if all( | ||
self._check_unsat(z3.And(*constraints), z3_condition) | ||
for edge in (pred for pred in node_block.predecessors if pred.is_feasible) | ||
for constraints in edge.z3_constraints.values() | ||
): | ||
self.add_message("impossible-condition", node=node.test) | ||
|
||
def _check_unsat(self, prev_constraints: z3.ExprRef, node_constraint: z3.ExprRef) -> bool: | ||
"""Check if the conjunction of the given constraints is unsatisfiable. | ||
|
||
- prev_constraints (z3.ExprRef): Constraints from previous nodes. | ||
- node_constraint (z3.ExprRef): The condition to check at the current node. | ||
""" | ||
solver = z3.Solver() | ||
solver.add(z3.And(prev_constraints, node_constraint)) | ||
return solver.check() == z3.unsat | ||
|
||
|
||
def register(linter: PyLinter) -> None: | ||
"""Required method to auto-register this checker to the linter.""" | ||
linter.register_checker(ConditionLogicChecker(linter)) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include a
from __future__ import annotations
, otherwise the type annotationz3.ExprRef
below will raise an error ifz3
is not installed