Include 'inside' expressions in logical-not-parentheses checking

MikePopoloski · Dec 15, 2024 · 7481d7f · 7481d7f
1 parent 10b0843
commit 7481d7f
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 1 deletion.
diff --git a/source/ast/expressions/OperatorExpressions.cpp b/source/ast/expressions/OperatorExpressions.cpp
@@ -1569,6 +1569,21 @@ Expression& InsideExpression::fromSyntax(Compilation& compilation,
     if (bad)
         return badExpr(compilation, result);
 
+    // Warn about `!x inside {y, z}` where they probably meant `!(x inside {y, z})`
+    auto& lhs = boundSpan[0]->unwrapImplicitConversions();
+    if (lhs.kind == ExpressionKind::UnaryOp && !lhs.isParenthesized() &&
+        lhs.as<UnaryExpression>().op == UnaryOperator::LogicalNot) {
+
+        auto& unary = lhs.as<UnaryExpression>();
+        auto kindStr = "'inside' expression"sv;
+        auto& diag = context.addDiag(diag::LogicalNotParentheses, unary.opRange);
+        diag << kindStr << syntax.inside.range();
+
+        SourceRange range(unary.operand().sourceRange.start(), result->sourceRange.end());
+        diag.addNote(diag::NoteLogicalNotFix, range) << kindStr;
+        diag.addNote(diag::NoteLogicalNotSilence, lhs.sourceRange);
+    }
+
     return *result;
 }
 

diff --git a/tests/unittests/ast/WarningTests.cpp b/tests/unittests/ast/WarningTests.cpp
@@ -1033,6 +1033,7 @@ module m;
         if ((a + b ? 1 : 2) == 2) begin end
         if (a < b < c) begin end
         c |= a & b;
+        if (!a inside {0, 1, 0}) begin end
     end
 endmodule
 )");
@@ -1041,7 +1042,7 @@ endmodule
     compilation.addSyntaxTree(tree);
 
     auto& diags = compilation.getAllDiagnostics();
-    REQUIRE(diags.size() == 10);
+    REQUIRE(diags.size() == 11);
     CHECK(diags[0].code == diag::BitwiseRelPrecedence);
     CHECK(diags[1].code == diag::BitwiseOpParentheses);
     CHECK(diags[2].code == diag::BitwiseOpParentheses);
@@ -1052,6 +1053,7 @@ endmodule
     CHECK(diags[7].code == diag::BitwiseOpMismatch);
     CHECK(diags[8].code == diag::ConditionalPrecedence);
     CHECK(diags[9].code == diag::ConsecutiveComparison);
+    CHECK(diags[10].code == diag::LogicalNotParentheses);
 }
 
 TEST_CASE("Case statement type warnings") {