port-name-suffix: handle `ref` direction and all-underscore names PortNameSuffixRule crashed the linter on two valid inputs: - A `ref` port direction: IsSuffixCorrect did suffixes.at(direction) on a map keyed only by input/output/inout, so a `ref` port (allowed by the grammar) threw std::out_of_range, uncaught, aborting the linter. Look the direction up with find() and treat an unknown direction as correct (no violation), matching Violation() which already ignores non-input/output/inout. - An all-underscore name (e.g. "_"): absl::StrSplit with SkipEmpty yields an empty parts list, and the size()<2 branch did not return, so name_parts.back() dereferenced an empty vector (UB). Return after reporting the violation; this also drops a redundant duplicate Violation for no-underscore names. Adds regression tests: a `ref` port is accepted, and an all-underscore port reports a single suffix violation instead of crashing. Signed-off-by: Eylon Krause <eylon1909@gmail.com>
diff --git a/verible/verilog/analysis/checkers/port-name-suffix-rule.cc b/verible/verilog/analysis/checkers/port-name-suffix-rule.cc index 85a0db3..3eaf392 100644 --- a/verible/verilog/analysis/checkers/port-name-suffix-rule.cc +++ b/verible/verilog/analysis/checkers/port-name-suffix-rule.cc
@@ -90,10 +90,14 @@ {"output", {"o", "no", "po"}}, {"inout", {"io", "nio", "pio"}}}; - // At this point it is guaranteed that the direction will be set to - // one of the expected values (used as keys in the map above). - // Therefore checking the suffix like this is safe - return suffixes.at(direction).count(suffix) == 1; + // `direction` is usually one of the map keys, but the grammar also permits a + // `ref` port direction, which has no suffix convention. Look the direction up + // safely and treat an unknown direction as "correct" (no violation), + // consistent with Violation() which also ignores non-input/output/inout + // directions. Using std::map::at() here would throw on e.g. "ref". + const auto it = suffixes.find(direction); + if (it == suffixes.end()) return true; + return it->second.count(suffix) == 1; } void PortNameSuffixRule::HandleSymbol(const Symbol &symbol, @@ -113,8 +117,11 @@ absl::StrSplit(name, '_', absl::SkipEmpty()); if (name_parts.size() < 2) { - // No suffix at all + // No suffix at all. This also covers an all-underscore name (e.g. "_"), + // for which SkipEmpty leaves name_parts empty; return here so the + // name_parts.back() access below is not reached on an empty vector. Violation(direction, token, context); + return; } if (!IsSuffixCorrect(name_parts.back(), direction)) {
diff --git a/verible/verilog/analysis/checkers/port-name-suffix-rule_test.cc b/verible/verilog/analysis/checkers/port-name-suffix-rule_test.cc index 69f18f4..db69d71 100644 --- a/verible/verilog/analysis/checkers/port-name-suffix-rule_test.cc +++ b/verible/verilog/analysis/checkers/port-name-suffix-rule_test.cc
@@ -57,6 +57,9 @@ {"module t (input bit name_i); endmodule;"}, {"module t (output bit abc_o); endmodule;"}, {"module t (inout bit xyz_io); endmodule;"}, + // A `ref` port has no suffix convention and must not be flagged (and must + // not crash the rule via std::map::at). + {"module t (ref logic data_x); endmodule;"}, {"module t (input logic name_i,\n" "output logic abc_o,\n" "inout logic xyz_io,\n" @@ -90,6 +93,10 @@ {"module t (output logic ", {kToken, "_o"}, "); endmodule;"}, {"module t (inout logic ", {kToken, "_io"}, "); endmodule;"}, + // An all-underscore name splits (SkipEmpty) to an empty parts list; it + // must report a suffix violation, not dereference an empty vector. + {"module t (input logic ", {kToken, "_"}, "); endmodule;"}, + {"module t (input logic ", {kToken, "namei"}, "); endmodule;"}, {"module t (input logic ", {kToken, "nam_ei"}, "); endmodule;"}, {"module t (input logic ", {kToken, "name_o"}, "); endmodule;"},