Address a few clang-tidy findings.
diff --git a/.clang-tidy b/.clang-tidy index e210319..be00837 100644 --- a/.clang-tidy +++ b/.clang-tidy
@@ -49,6 +49,7 @@ -readability-isolate-declaration, -readability-magic-numbers, -readability-make-member-function-const, + -readability-math-missing-parentheses, -readability-named-parameter, -readability-qualified-auto, -readability-redundant-access-specifiers,
diff --git a/verible/common/formatting/layout-optimizer-internal.h b/verible/common/formatting/layout-optimizer-internal.h index cd489f7..ed59fe7 100644 --- a/verible/common/formatting/layout-optimizer-internal.h +++ b/verible/common/formatting/layout-optimizer-internal.h
@@ -617,7 +617,7 @@ // // Iterator: iterator type that dereferences to LayoutFunction. template <class Iterator> - LayoutFunction Choice(const Iterator begin, const Iterator end) const { + LayoutFunction Choice(const Iterator &begin, const Iterator &end) const { static_assert(IsIteratorDereferencingTo<Iterator, LayoutFunction>, "Iterator's value type must be LayoutFunction."); const auto lfs = make_container_range(begin, end);
diff --git a/verible/common/lsp/message-stream-splitter.cc b/verible/common/lsp/message-stream-splitter.cc index 3daf864..a3c07d9 100644 --- a/verible/common/lsp/message-stream-splitter.cc +++ b/verible/common/lsp/message-stream-splitter.cc
@@ -48,12 +48,12 @@ static constexpr std::string_view kEndHeaderMarker = "\r\n\r\n"; static constexpr std::string_view kContentLengthHeader = "Content-Length: "; - int header_marker_len = kEndHeaderMarker.length(); - auto end_of_header = data.find(kEndHeaderMarker); + const size_t header_marker_len = kEndHeaderMarker.length(); + const size_t end_of_header = data.find(kEndHeaderMarker); if (end_of_header == std::string_view::npos) return kIncompleteHeader; // Very dirty search for header - we don't check if starts with line. - const std::string_view header_content(data.data(), end_of_header); + const std::string_view header_content = data.substr(0, end_of_header); auto found_ContentLength_header = header_content.find(kContentLengthHeader); if (found_ContentLength_header == std::string_view::npos) { return kGarbledHeader; @@ -80,8 +80,8 @@ int body_size = 0; const int body_offset = ParseHeaderGetBodyOffset(*data, &body_size); if (body_offset == kGarbledHeader) { - std::string_view limited_view( - data->data(), std::min(data->size(), static_cast<size_t>(256))); + const std::string_view limited_view = + data->substr(0, std::min(data->size(), static_cast<size_t>(256))); return absl::InvalidArgumentError( absl::StrCat("No `Content-Length:` header. '", absl::CEscape(limited_view), "...'")); @@ -93,8 +93,8 @@ return absl::OkStatus(); // Only insufficient partial buffer available. } - std::string_view header(data->data(), body_offset); - std::string_view body(data->data() + body_offset, body_size); + const std::string_view header = data->substr(0, body_offset); + const std::string_view body = data->substr(body_offset, body_size); message_processor_(header, body); stats_largest_body_ = std::max(stats_largest_body_, body.size());
diff --git a/verible/common/text/macro-definition.cc b/verible/common/text/macro-definition.cc index d1a32aa..7f795f0 100644 --- a/verible/common/text/macro-definition.cc +++ b/verible/common/text/macro-definition.cc
@@ -104,7 +104,7 @@ } } // Didn't match enum type nor find map entry, so don't substitute. - return token_info; + return token_info; // NOLINT(bugprone-return-const-ref-from-parameter) } } // namespace verible
diff --git a/verible/common/text/tree-utils.h b/verible/common/text/tree-utils.h index a69cfaa..8a18924 100644 --- a/verible/common/text/tree-utils.h +++ b/verible/common/text/tree-utils.h
@@ -54,11 +54,9 @@ // The following no-op overloads allow SymbolCastToNode() to work with zero // overhead when the argument type is statically known to be the same. inline const SyntaxTreeNode &SymbolCastToNode(const SyntaxTreeNode &node) { - return node; + return node; // NOLINT(bugprone-return-const-ref-from-parameter) } -inline SyntaxTreeNode &SymbolCastToNode(SyntaxTreeNode &node) { // NOLINT - return node; -} +inline SyntaxTreeNode &SymbolCastToNode(SyntaxTreeNode &node) { return node; } // Returns a SyntaxTreeLeaf down_casted from a Symbol. const SyntaxTreeLeaf &SymbolCastToLeaf(const Symbol &);
diff --git a/verible/common/util/container-proxy_test.cc b/verible/common/util/container-proxy_test.cc index b5766fd..5e8220f 100644 --- a/verible/common/util/container-proxy_test.cc +++ b/verible/common/util/container-proxy_test.cc
@@ -113,7 +113,7 @@ std::equal(values.begin(), values.end(), other.values.begin()); } - friend std::ostream &operator<<(std::ostream &s, TestTrace obj) { + friend std::ostream &operator<<(std::ostream &s, const TestTrace &obj) { switch (obj.triggered_method) { case ContainerProxyEvent::kUnknown: s << "UNKNOWN";
diff --git a/verible/common/util/container-util.h b/verible/common/util/container-util.h index b27f12c..85bfa91 100644 --- a/verible/common/util/container-util.h +++ b/verible/common/util/container-util.h
@@ -43,7 +43,8 @@ template <class KeyType> struct ValueTypeFromKeyType<KeyType, KeyType> { const KeyType &operator()(const KeyType &k) const { - return k; // just forward the key + // Just forward the key + return k; // NOLINT(bugprone-return-const-ref-from-parameter) } }; } // namespace internal
diff --git a/verible/common/util/file-util.cc b/verible/common/util/file-util.cc index bd94cf6..27a4c6f 100644 --- a/verible/common/util/file-util.cc +++ b/verible/common/util/file-util.cc
@@ -179,10 +179,10 @@ } char buffer[4096]; int bytes_read; - do { - bytes_read = fread(buffer, 1, sizeof(buffer), stream); + while (!ferror(stream) && !feof(stream) && + (bytes_read = fread(buffer, 1, sizeof(buffer), stream)) >= 0) { content.append(buffer, bytes_read); - } while (bytes_read > 0); + } fclose(stream); return content;
diff --git a/verible/common/util/forward.h b/verible/common/util/forward.h index 9778108..56af125 100644 --- a/verible/common/util/forward.h +++ b/verible/common/util/forward.h
@@ -31,7 +31,9 @@ // Intentionally restricted to const reference to avoid the surprise // of modifying a temporary reference. // See also the two-parameter overload. - const T &operator()(const T &t) const { return t; } + const T &operator()(const T &t) const { + return t; // NOLINT(bugprone-return-const-ref-from-parameter) + } // (overload) Constructs a temporary rvalue, when types are different. // This works with explicit-only constructors and implicit constructors.
diff --git a/verible/verilog/analysis/checkers/constraint-name-style-rule.cc b/verible/verilog/analysis/checkers/constraint-name-style-rule.cc index c6be316..0c66ab9 100644 --- a/verible/verilog/analysis/checkers/constraint-name-style-rule.cc +++ b/verible/verilog/analysis/checkers/constraint-name-style-rule.cc
@@ -51,7 +51,7 @@ .desc = "Check that constraint names follow the required name style " "specified by a regular expression.", - .param = {{"pattern", kSuffix.data()}}, + .param = {{"pattern", std::string(kSuffix)}}, }; return d; }
diff --git a/verible/verilog/analysis/verilog-linter-configuration.cc b/verible/verilog/analysis/verilog-linter-configuration.cc index 489059a..07cbef4 100644 --- a/verible/verilog/analysis/verilog-linter-configuration.cc +++ b/verible/verilog/analysis/verilog-linter-configuration.cc
@@ -281,7 +281,8 @@ template <typename T> static absl::StatusOr<std::vector<std::unique_ptr<T>>> CreateRules( const std::map<analysis::LintRuleId, RuleSetting> &config, - std::function<std::unique_ptr<T>(const analysis::LintRuleId &)> factory) { + const std::function<std::unique_ptr<T>(const analysis::LintRuleId &)> + &factory) { std::vector<std::unique_ptr<T>> rule_instances; for (const auto &rule_pair : config) { const RuleSetting &setting = rule_pair.second;