Skip to content

Commit

Permalink
feat: continue work on better detection of if statements by ifstateme…
Browse files Browse the repository at this point in the history
…nts rule

FossilOrigin-Name: cadf3dee9480ba02ad7e3d0feae81094b872a1a7fe40f979d0fa26810b688a99
  • Loading branch information
thindil committed Mar 17, 2024
1 parent bd5f941 commit f2bbde9
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 16 deletions.
52 changes: 37 additions & 15 deletions src/rules/ifstatements.nim
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,32 @@ proc checkMoveableBranch(node, parent: PNode; messagePrefix: string;
ruleData = "outside", params = [$node.info.line,
"the content of the last branch can" & negation &
" be moved outside the if statement."])

proc checkNegativeCondition(node, parent: PNode; messagePrefix: string;
rule: var RuleOptions) {.raises: [KeyError, Exception], tags: [RootEffect], contractual.} =
## Check the if statement for empty branches
##
## * node - the node which will be checked
## * parent - the parent node of the node to check
## * messagePrefix - the prefix added to the log message, set by the program
## * rule - the rule options set by the user
body:
let astNode: PNode = parent
let conditions: seq[string] = ($node[0]).split
var checkResult: bool = true
if (conditions.len > 2 and conditions[2] == "not") or (
conditions.len > 3 and conditions[3] in ["notin", "!="]):
checkResult = node[^1].kind notin {nkElse, nkElseExpr}
if rule.ruleType == RuleTypes.count:
checkResult = not checkResult
elif rule.ruleType in {RuleTypes.count, search}:
checkResult = false
setResult(checkResult = checkResult,
positiveMessage = positiveMessage,
negativeMessage = negativeMessage, node = node,
ruleData = "negation", params = [$node.info.line,
(if rule.negation: "doesn't start" else: "starts") &
" with a negative condition."])
{.pop ruleOff: "paramsUsed".}

checkRule:
Expand All @@ -227,21 +253,8 @@ checkRule:
# Check if the if statement starts with negative condition and has else branch
if rule.options[0].toLowerAscii in ["all", "negative"]:
try:
let conditions: seq[string] = ($node[0]).split
var checkResult: bool = true
if (conditions.len > 2 and conditions[2] == "not") or (
conditions.len > 3 and conditions[3] in ["notin", "!="]):
checkResult = node[^1].kind notin {nkElse, nkElseExpr}
if rule.ruleType == RuleTypes.count:
checkResult = not checkResult
elif rule.ruleType in {RuleTypes.count, search}:
checkResult = false
setResult(checkResult = checkResult,
positiveMessage = positiveMessage,
negativeMessage = negativeMessage, node = node,
ruleData = "negation", params = [$node.info.line,
(if rule.negation: "doesn't start" else: "starts") &
" with a negative condition."])
checkNegativeCondition(node = node, parent = parentNode,
messagePrefix = messagePrefix, rule = rule)
except Exception as e:
rule.amount = errorMessage(
text = "Can't check the if statement.", e = e)
Expand All @@ -268,6 +281,15 @@ checkRule:
if child.kind in {nkIfStmt, nkElifBranch, nkWhenStmt}:
var oldAmount: int = rule.amount
if node.len > 1:
# Check if the if statement starts with negative condition and has else branch
if rule.options[0].toLowerAscii in ["all", "negative"]:
try:
checkNegativeCondition(node = node, parent = parentNode,
messagePrefix = messagePrefix, rule = rule)
except Exception as e:
rule.amount = errorMessage(
text = "Can't check the if statement.", e = e)
return
# Check if the last if branch can be moved outside the if statement
if rule.options[0].toLowerAscii in ["all", "moveable"] and
rule.amount == oldAmount:
Expand Down
2 changes: 1 addition & 1 deletion tests/invalid/ifstatements.nim
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ if a == 0:

if a > 0:
for i in 1 .. 5:
if a == 1:
if a != 1:
discard
continue

0 comments on commit f2bbde9

Please sign in to comment.