forbid line continuations inside string-literals (#674)

* forbid line continuations inside string-literals
Adding:
 - TokenStreamLintRule definition for linter
 - tests for TokenStreamLintRule

Fixes #384 

Signed-off-by: Pawel Sagan <[email protected]>

verilog/CST/verilog_matchers.h

@@ -298,6 +298,10 @@ static const auto AlwaysCombKeyword =
 static const auto AlwaysFFKeyword =
     verible::matcher::MakePathMatcher({L(TK_always_ff)});
 
+// Matches occurrence of the 'StringLiteral' keyword.
+static const auto StringLiteralKeyword =
+    verible::matcher::MakePathMatcher({L(TK_StringLiteral)});
+
 // Matches legacy-style begin-block inside generate region.
 //
 // For instance, matches:

verilog/analysis/checkers/BUILD

@@ -58,6 +58,7 @@ cc_library(
         ":proper_parameter_declaration_rule",
         ":signal_name_style_rule",
         ":struct_union_name_style_rule",
+        ":token_stream_lint_rule",
         ":suggest_parentheses_rule",
         ":undersized_binary_literal_rule",
         ":unpacked_dimensions_rule",
@@ -1071,6 +1072,42 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "token_stream_lint_rule",
+    srcs = ["token_stream_lint_rule.cc"],
+    hdrs = ["token_stream_lint_rule.h"],
+    deps = [
+        "//common/analysis:citation",
+        "//common/analysis:lint_rule_status",
+        "//common/analysis:syntax_tree_lint_rule",
+        "//common/analysis/matcher",
+        "//common/analysis/matcher:bound_symbol_manager",
+        "//common/text:symbol",
+        "//common/text:syntax_tree_context",
+        "//verilog/CST:verilog_matchers",
+        "//verilog/analysis:descriptions",
+        "//verilog/analysis:lint_rule_registry",
+        "@com_google_absl//absl/strings",
+    ],
+    alwayslink = 1,
+)
+
+cc_test(
+    name = "token_stream_lint_rule_test",
+    srcs = ["token_stream_lint_rule_test.cc"],
+    deps = [
+        ":token_stream_lint_rule",
+        "//common/analysis:linter_test_utils",
+        "//common/analysis:syntax_tree_linter_test_utils",
+        "//common/text:symbol",
+        "//verilog/CST:verilog_nonterminals",
+        "//verilog/CST:verilog_treebuilder_utils",
+        "//verilog/analysis:verilog_analyzer",
+        "//verilog/parser:verilog_token_enum",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "suggest_parentheses_rule",
     srcs = ["suggest_parentheses_rule.cc"],

verilog/analysis/checkers/token_stream_lint_rule.cc

@@ -0,0 +1,83 @@
+// 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/token_stream_lint_rule.h"
+
+#include <set>
+#include <string>
+
+#include "absl/strings/str_cat.h"
+#include "absl/strings/string_view.h"
+#include "common/analysis/citation.h"
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/matcher/bound_symbol_manager.h"
+#include "common/analysis/matcher/matcher.h"
+#include "common/text/symbol.h"
+#include "common/text/syntax_tree_context.h"
+#include "verilog/CST/verilog_matchers.h"  // IWYU pragma: keep
+#include "verilog/analysis/descriptions.h"
+#include "verilog/analysis/lint_rule_registry.h"
+
+namespace verilog {
+namespace analysis {
+
+using verible::GetStyleGuideCitation;
+using verible::LintRuleStatus;
+using verible::LintViolation;
+using verible::SyntaxTreeContext;
+using verible::matcher::Matcher;
+
+// Register TokenStreamLintRule
+VERILOG_REGISTER_LINT_RULE(TokenStreamLintRule);
+
+absl::string_view TokenStreamLintRule::Name() {
+  return "forbid-line-continuations";
+}
+const char TokenStreamLintRule::kTopic[] = "forbid-line-continuations";
+const char TokenStreamLintRule::kMessage[] =
+    "The lines can't be continued with \'\\\', use concatenation operator with "
+    "braces";
+
+std::string TokenStreamLintRule::GetDescription(
+    DescriptionType description_type) {
+  return absl::StrCat("Checks that there are no occurrences of ",
+                      Codify("\'\\\'", description_type),
+                      " when breaking the string literal line.",
+                      "Use concatenation operator with braces instead. See ",
+                      GetStyleGuideCitation(kTopic), ".");
+}
+
+static const Matcher& StringLiteralMatcher() {
+  static const Matcher matcher(StringLiteralKeyword());
+  return matcher;
+}
+
+void TokenStreamLintRule::HandleSymbol(const verible::Symbol& symbol,
+                                       const SyntaxTreeContext& context) {
+  verible::matcher::BoundSymbolManager manager;
+  if (StringLiteralMatcher().Matches(symbol, &manager)) {
+    const auto& string_node = SymbolCastToNode(symbol);
+    const auto& string_literal = SymbolCastToLeaf(*string_node.children()[0]);
+    if (string_literal.get().text().find("\\\n") != std::string::npos) {
+      violations_.insert(LintViolation(symbol, kMessage, context));
+    }
+  }
+}
+
+LintRuleStatus TokenStreamLintRule::Report() const {
+  return LintRuleStatus(violations_, Name(), GetStyleGuideCitation(kTopic));
+}
+
+}  // namespace analysis
+}  // namespace verilog

verilog/analysis/checkers/token_stream_lint_rule.h

@@ -0,0 +1,58 @@
+// 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.
+
+#ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_TOKEN_STREAM_LINT_RULE_H_
+#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_TOKEN_STREAM_LINT_RULE_H_
+
+#include <set>
+#include <string>
+
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/syntax_tree_lint_rule.h"
+#include "common/text/symbol.h"
+#include "common/text/syntax_tree_context.h"
+#include "verilog/analysis/descriptions.h"
+
+namespace verilog {
+namespace analysis {
+
+// TokenStreamLintRule finds occurrences of any string literal.
+class TokenStreamLintRule : public verible::SyntaxTreeLintRule {
+ public:
+  using rule_type = verible::SyntaxTreeLintRule;
+  static absl::string_view Name();
+
+  // Returns the description of the rule implemented formatted for either the
+  // helper flag or markdown depending on the parameter type.
+  static std::string GetDescription(DescriptionType);
+
+  void HandleSymbol(const verible::Symbol& symbol,
+                    const verible::SyntaxTreeContext& context) override;
+
+  verible::LintRuleStatus Report() const override;
+
+ private:
+  // Link to style guide rule.
+  static const char kTopic[];
+
+  // Diagnostic message.
+  static const char kMessage[];
+
+  std::set<verible::LintViolation> violations_;
+};
+
+}  // namespace analysis
+}  // namespace verilog
+
+#endif  // VERIBLE_VERILOG_ANALYSIS_CHECKERS_TOKEN_STREAM_LINT_RULE_H_

verilog/analysis/checkers/token_stream_lint_rule_test.cc

@@ -0,0 +1,65 @@
+// 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/token_stream_lint_rule.h"
+
+#include <initializer_list>
+
+#include "common/analysis/linter_test_utils.h"
+#include "common/analysis/syntax_tree_linter_test_utils.h"
+#include "common/text/symbol.h"
+#include "gtest/gtest.h"
+#include "verilog/CST/verilog_nonterminals.h"
+#include "verilog/CST/verilog_treebuilder_utils.h"
+#include "verilog/analysis/verilog_analyzer.h"
+#include "verilog/parser/verilog_token_enum.h"
+
+namespace verilog {
+namespace analysis {
+namespace {
+
+using verible::LintTestCase;
+using verible::RunLintTestCases;
+
+TEST(StringLiteralConcatenationTest, FunctionFailures) {
+  constexpr int kToken = TK_StringLiteral;
+  const std::initializer_list<LintTestCase> kStringLiteralTestCases = {
+      {""},
+      {"module m;\nendmodule\n"},
+      {"module m;\n", "string tmp = {\"Humpty Dumpty sat on a wall.\",",
+       "\"Humpty Dumpty had a great fall.\"};", "\nendmodule"},
+      {"module m;\n", "string tmp = {\"Humpty Dumpty \\ sat on a wall.\",",
+       "\"Humpty Dumpty had a great fall.\"};", "\nendmodule"},
+      {"module m;\n",
+       "string tmp=",
+       {kToken,
+        "\"Humpty Dumpty sat on a wall. \\\nHumpty Dumpty had a great fall.\""},
+       ";",
+       "\nendmodule"},
+      {"module m;\n",
+       "string tmp=",
+       {kToken,
+        "\"Humpty Dumpty sat on a wall. \\\n\\\nHumpty Dumpty had a great "
+        "fall.\""},
+       ";",
+       "\nendmodule"},
+  };
+
+  RunLintTestCases<VerilogAnalyzer, TokenStreamLintRule>(
+      kStringLiteralTestCases);
+}
+
+}  // namespace
+}  // namespace analysis
+}  // namespace verilog

verilog/analysis/default_rules.h

@@ -30,6 +30,7 @@ constexpr const char* kDefaultRuleSet[] = {
     "module-filename",
     "package-filename",
     "void-cast",
+    "forbid-line-continuations",
     "generate-label",
     "generate-label-prefix",
     "always-comb",

verilog/tools/lint/BUILD

@@ -30,6 +30,7 @@ _linter_test_configs = [
     ("explicit-parameter-storage-type", "explicit_parameter_storage_type", True),
     ("explicit-task-lifetime", "explicit_task_lifetime", True),
     ("forbid-consecutive-null-statements", "forbid_consecutive_null_statements", True),
+    ("forbid-line-continuations", "forbid_line_continuations", True),
     ("generate-label", "generate_label_module", True),
     ("generate-label", "generate-label-module-body", True),  # uses parse directive
     ("generate-label-prefix", "generate_label_prefix", True),

verilog/tools/lint/testdata/forbid_line_continuations.sv

@@ -0,0 +1,9 @@
+module forbid_line_continuations;
+
+string bad_literal = "Humpty Dumpty sat on a wall. \
+Humpty Dumpty had a great fall.";
+
+string good_literal = {"Humpty Dumpty sat on a wall.",
+"Humpty Dumpty had a great fall."};
+
+endmodule