diff --git a/verilog/analysis/checkers/explicit_begin_rule.cc b/verilog/analysis/checkers/explicit_begin_rule.cc index 690e381ee..3f38494cf 100644 --- a/verilog/analysis/checkers/explicit_begin_rule.cc +++ b/verilog/analysis/checkers/explicit_begin_rule.cc @@ -42,7 +42,7 @@ using verible::TokenStreamLintRule; VERILOG_REGISTER_LINT_RULE(ExplicitBeginRule); static const char kMessage[] = - "All block construct shall explicitely use begin/end"; + "All block construct shall explicitely use begin/end."; const LintRuleDescriptor& ExplicitBeginRule::GetDescriptor() { static const LintRuleDescriptor d{ @@ -89,6 +89,7 @@ void ExplicitBeginRule::HandleToken(const TokenInfo& token) { break; case TK_COMMENT_BLOCK: case TK_EOL_COMMENT: + case TK_NEWLINE: break; case TK_if: case TK_for: @@ -123,6 +124,7 @@ void ExplicitBeginRule::HandleToken(const TokenInfo& token) { break; case TK_COMMENT_BLOCK: case TK_EOL_COMMENT: + case TK_NEWLINE: break; case TK_begin: // If we got our begin token, we go back to normal status @@ -139,7 +141,7 @@ void ExplicitBeginRule::HandleToken(const TokenInfo& token) { if (raise_violation) { violations_.insert(LintViolation( - last_condition_start_, absl::StrCat(kMessage), + token, absl::StrCat(kMessage, " Expected begin, got ", token.text()), {AutoFix( "Insert begin", {end_of_condition_statement_, diff --git a/verilog/analysis/checkers/explicit_begin_rule.h b/verilog/analysis/checkers/explicit_begin_rule.h index ad08a195f..ae1d67406 100644 --- a/verilog/analysis/checkers/explicit_begin_rule.h +++ b/verilog/analysis/checkers/explicit_begin_rule.h @@ -31,18 +31,27 @@ namespace analysis { // matches the opening `ifdef or `ifndef. // // Accepted examples: -// `ifdef FOO -// `endif // FOO +// if (FOO) begin // -// `ifndef BAR -// `endif // BAR +// else begin +// +// else if (FOO) begin +// +// for (FOO) begin // // Rejected examples: -// `ifdef FOO -// `endif +// if (FOO) +// BAR +// +// else +// BAR +// +// else if (FOO) +// BAR +// +// for (FOO) begin +// BAR // -// `ifdef FOO -// `endif // BAR // class ExplicitBeginRule : public verible::TokenStreamLintRule { public: @@ -58,12 +67,7 @@ class ExplicitBeginRule : public verible::TokenStreamLintRule { private: // States of the internal token-based analysis. - enum class State { - kNormal, - kInCondition, - kInElse, - kExpectBegin - }; + enum class State { kNormal, kInCondition, kInElse, kExpectBegin }; // Internal lexical analysis state. State state_; @@ -73,8 +77,8 @@ class ExplicitBeginRule : public verible::TokenStreamLintRule { // Token information for the last seen block opening (for/if/else). verible::TokenInfo last_condition_start_ = verible::TokenInfo::EOFToken(); - verible::TokenInfo end_of_condition_statement_ = verible::TokenInfo::EOFToken(); - + verible::TokenInfo end_of_condition_statement_ = + verible::TokenInfo::EOFToken(); // Collection of found violations. std::set violations_; diff --git a/verilog/analysis/checkers/explicit_begin_rule_test.cc b/verilog/analysis/checkers/explicit_begin_rule_test.cc new file mode 100644 index 000000000..025678996 --- /dev/null +++ b/verilog/analysis/checkers/explicit_begin_rule_test.cc @@ -0,0 +1,101 @@ +// Copyright 2017-2020 The Verible Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "verilog/analysis/checkers/explicit_begin_rule.h" + +#include + +#include "common/analysis/linter_test_utils.h" +#include "common/analysis/token_stream_linter_test_utils.h" +#include "common/text/symbol.h" +#include "gtest/gtest.h" +#include "verilog/analysis/verilog_analyzer.h" +#include "verilog/parser/verilog_token_enum.h" + +namespace verilog { +namespace analysis { +namespace { + +using verible::LintTestCase; +using verible::RunApplyFixCases; +using verible::RunLintTestCases; + +// Tests that space-only text passes. +TEST(ExplicitBeginRuleTest, AcceptsBlank) { + const std::initializer_list kTestCases = { + {""}, + {" "}, + {"\n"}, + {" \n\n"}, + }; + RunLintTestCases(kTestCases); +} + +// Tests that properly matched if/begin passes. +TEST(ExplicitBeginRuleTest, AcceptsEndifWithComment) { + const std::initializer_list kTestCases = { + {"if (FOO) begin"}, + {"if (FOO)\n begin"}, + {"if (FOO) //Comment\n begin"}, + {"else begin \n FOO"}, + {"else \nbegin \n FOO"}, + {"else //Comment\n begin \n FOO"}, + {"else \n //Comment\n begin \n FOO"}, + {"else if (FOO) begin"}, + {"else if (FOO)\n begin"}, + {"else if (FOO) //Comment\n begin"}, + {"else if (FOO)\n //Comment\n begin"}, + }; + RunLintTestCases(kTestCases); +} + +// Tests that unmatched block/begin fails is detected. +TEST(ExplicitBeginRuleTest, RejectsEndifWithoutComment) { + const std::initializer_list kTestCases = { + {"if (FOO) BAR"}, + {"if (FOO)\n BAR"}, + {"if (FOO) //Comment\n BAR"}, + {"else \n FOO"}, + {"else \n \n FOO"}, + {"else //Comment\n FOO"}, + {"else \n //Comment\n FOO"}, + {"else if (FOO) BAR"}, + {"else if (FOO)\n BAR"}, + {"else if (FOO) //Comment\n BAR"}, + {"else if (FOO)\n //Comment\n BAR"}, + }; + RunLintTestCases(kTestCases); +} + +TEST(ExplicitBeginRuleTest, ApplyAutoFix) { + // Alternatives the auto fix offers + const std::initializer_list kTestCases = { + {"if (FOO) BAR","if (FOO) begin BAR"}, + {"if (FOO)\n BAR","if (FOO) begin\n BAR"}, + {"if (FOO) //Comment\n BAR","if (FOO) begin //Comment\n BAR"}, + {"else \n FOO","else begin \n FOO"}, + {"else \n \n FOO","else begin \n \n FOO"}, + {"else //Comment\n FOO","else begin //Comment\n FOO"}, + {"else \n //Comment\n FOO","else begin \n //Comment\n FOO"}, + {"else if (FOO) BAR","else if (FOO) begin BAR"}, + {"else if (FOO)\n BAR","else if (FOO) begin\n BAR"}, + {"else if (FOO) //Comment\n BAR","else if (FOO) begin //Comment\n BAR"}, + {"else if (FOO)\n //Comment\n BAR","else if (FOO) begin\n //Comment\n BAR"}, + }; + RunApplyFixCases(kTestCases, ""); +} + +} // namespace +} // namespace analysis +} // namespace verilog