style(clang-tidy): apply more fixes found by clang-tidy

export/planloader/planloader.cpp

@@ -24,7 +24,7 @@ SerializedPlan* load_substrait_plan(const char* filename) {
     strncpy(newPlan->error_message, errMsg.data(), errMsg.length() + 1);
     return newPlan;
   }
-  ::substrait::proto::Plan plan = *planOrError;
+  const ::substrait::proto::Plan& plan = *planOrError;
   std::string text = plan.SerializeAsString();
   newPlan->buffer = new unsigned char[text.length() + 1];
   memcpy(newPlan->buffer, text.data(), text.length() + 1);

src/substrait/common/PlanTransformerTool.cpp

@@ -38,15 +38,15 @@ int main(int argc, char* argv[]) {
 
   auto planOrError = io::substrait::loadPlan(argv[1]);
   if (!planOrError.ok()) {
-    std::cerr << planOrError.status() << std::endl;
+    std::cerr << planOrError.status() << '\n';
     return EXIT_FAILURE;
   }
 
   auto format = io::substrait::planFileFormatFromText(argv[3]);
 
   auto result = io::substrait::savePlan(*planOrError, argv[2], format);
   if (!result.ok()) {
-    std::cerr << result << std::endl;
+    std::cerr << result << '\n';
     return EXIT_FAILURE;
   }
 

src/substrait/expression/DecimalLiteral.cpp

@@ -56,7 +56,10 @@ DecimalLiteral DecimalLiteral::fromString(
   }
   std::uint8_t valueBytes[16];
   uint128ToBytes(v, valueBytes);
-  return {std::string((const char*)valueBytes, 16), precision, scale};
+  return {
+      std::string(reinterpret_cast<const char*>(valueBytes), 16),
+      precision,
+      scale};
 }
 
 bool DecimalLiteral::isValid() {

src/substrait/textplan/SymbolTable.cpp

@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: Apache-2.0 */
 #include "substrait/textplan/SymbolTable.h"
 
+#include <algorithm>
 #include <any>
 #include <iomanip>
 #include <map>
@@ -27,7 +28,7 @@ const std::string& symbolTypeName(SymbolType type) {
       "kSourceDetail",
       "kField",
   };
-  auto typeNum = int32_t(type);
+  auto typeNum = static_cast<int32_t>(type);
   if (typeNum == -1) {
     static std::string unknown = "kUnknown";
     return unknown;
@@ -136,9 +137,7 @@ void SymbolTable::setParentQueryLocation(
   int highestIndex = -1;
   for (const auto& sym : symbols_) {
     if (sym->parentQueryLocation == location) {
-      if (sym->parentQueryIndex > highestIndex) {
-        highestIndex = sym->parentQueryIndex;
-      }
+      highestIndex = std::max(sym->parentQueryIndex, highestIndex);
     }
   }
   symbols_[index]->parentQueryIndex = highestIndex + 1;
@@ -236,7 +235,7 @@ std::string SymbolTable::toDebugString() const {
     if (!relationData->subQueryPipelines.empty()) {
       result << " SQC=" << relationData->subQueryPipelines.size();
     }
-    result << std::endl;
+    result << '\n';
 
     int32_t fieldNum = 0;
     for (const auto& field : relationData->fieldReferences) {
@@ -248,7 +247,7 @@ std::string SymbolTable::toDebugString() const {
       if (!field->alias.empty()) {
         result << " " << field->alias;
       }
-      result << std::endl;
+      result << '\n';
     }
 
     for (const auto& field : relationData->generatedFieldReferences) {
@@ -266,7 +265,7 @@ std::string SymbolTable::toDebugString() const {
       } else if (!field->alias.empty()) {
         result << " " << field->alias;
       }
-      result << std::endl;
+      result << '\n';
     }
 
     int32_t outputFieldNum = 0;
@@ -279,12 +278,12 @@ std::string SymbolTable::toDebugString() const {
       if (!field->alias.empty()) {
         result << " " << field->alias;
       }
-      result << std::endl;
+      result << '\n';
     }
     textAlreadyWritten = true;
   }
   if (textAlreadyWritten) {
-    result << std::endl;
+    result << '\n';
   }
   return result.str();
 }

src/substrait/textplan/converter/Tool.cpp

@@ -17,7 +17,7 @@ namespace {
 void convertPlanToText(const char* filename) {
   auto planOrError = loadPlan(filename);
   if (!planOrError.ok()) {
-    std::cerr << planOrError.status() << std::endl;
+    std::cerr << planOrError.status() << '\n';
     return;
   }
 
@@ -29,7 +29,7 @@ void convertPlanToText(const char* filename) {
   auto errors = result.getAllErrors();
   if (!errors.empty()) {
     for (const auto& err : errors) {
-      std::cerr << err << std::endl;
+      std::cerr << err << '\n';
     }
   }
   std::cout << textResult;

src/substrait/textplan/converter/tests/BinaryToTextPlanConversionTest.cpp

@@ -685,7 +685,7 @@ TEST_F(BinaryToTextPlanConversionTest, FullSample) {
   ASSERT_TRUE(jsonOrError.ok());
   auto planOrError = loadFromJson(*jsonOrError);
   ASSERT_TRUE(planOrError.ok());
-  auto plan = *planOrError;
+  const auto& plan = *planOrError;
   EXPECT_THAT(plan.extensions_size(), ::testing::Eq(7));
 
   auto expectedOutputOrError = readFromFile("data/q6_first_stage.golden.splan");

src/substrait/textplan/parser/ParseText.cpp

@@ -23,11 +23,11 @@ std::optional<antlr4::ANTLRInputStream> loadTextFile(
     std::string_view filename) {
   std::ifstream stream(std::string{filename});
   if (stream.bad() || stream.fail()) {
-    std::cout << "Bad stream." << std::endl;
+    std::cout << "Bad stream." << '\n';
     return std::nullopt;
   }
   if (!stream.is_open()) {
-    std::cout << "Stream is not open." << std::endl;
+    std::cout << "Stream is not open." << '\n';
     return std::nullopt;
   }
   return {stream};

src/substrait/textplan/parser/Tool.cpp

@@ -12,13 +12,13 @@ namespace {
 void readText(const char* filename) {
   auto stream = io::substrait::textplan::loadTextFile(filename);
   if (!stream.has_value()) {
-    std::cerr << "An error occurred while reading: " << filename << std::endl;
+    std::cerr << "An error occurred while reading: " << filename << '\n';
     return;
   }
   auto parseResult = io::substrait::textplan::parseStream(&*stream);
   if (!parseResult.successful()) {
     for (const std::string& msg : parseResult.getAllErrors()) {
-      std::cout << msg << std::endl;
+      std::cout << msg << '\n';
     }
     return;
   }
@@ -28,7 +28,7 @@ void readText(const char* filename) {
       parseResult.getSymbolTable(), &errorListener);
   if (errorListener.hasErrors()) {
     for (const std::string& msg : errorListener.getErrorMessages()) {
-      std::cout << msg << std::endl;
+      std::cout << msg << '\n';
     }
     return;
   }

src/substrait/textplan/tests/ParseResultMatchers.cpp

@@ -56,7 +56,7 @@ bool stringEqSquashingWhitespace(
                   << have.substr(atHave - have.begin(), 20) << "|"
                   << " should be |"
                   << expected.substr(atExpected - expected.begin(), 20) << "|"
-                  << std::endl;
+                  << '\n';
       }
       return false;
     }
@@ -164,7 +164,7 @@ class HasSymbolsMatcher {
           expectedSymbolsSorted.end(),
           std::back_inserter(extraSymbols));
       if (!extraSymbols.empty()) {
-        *listener << std::endl << "          with extra symbols: ";
+        *listener << '\n' << "          with extra symbols: ";
         for (const auto& symbol : extraSymbols) {
           *listener << " \"" << symbol << "\"";
         }
@@ -314,7 +314,7 @@ class HasSymbolsWithTypesMatcher {
       std::vector<std::string> extraSymbols =
           orderedSetDifference(actualSymbols, expectedSymbols_);
       if (!extraSymbols.empty()) {
-        *listener << std::endl << "          with extra symbols";
+        *listener << '\n' << "          with extra symbols";
         DescribeTypes(listener);
         *listener << ": ";
         for (const auto& symbol : extraSymbols) {

src/substrait/textplan/tests/RoundtripTest.cpp

@@ -38,7 +38,7 @@ std::string addLineNumbers(const std::string& text) {
   int lineNum = 0;
   std::string line;
   while (std::getline(input, line, '\n')) {
-    result << std::setw(4) << ++lineNum << " " << line << std::endl;
+    result << std::setw(4) << ++lineNum << " " << line << '\n';
   }
   return result.str();
 }
@@ -72,13 +72,13 @@ std::vector<std::string> getTestCases() {
 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(RoundTripBinaryToTextFixture);
 
 TEST_P(RoundTripBinaryToTextFixture, RoundTrip) {
-  auto filename = GetParam();
+  const auto& filename = GetParam();
   auto jsonOrError = readFromFile(filename);
   ASSERT_TRUE(jsonOrError.ok());
   auto planOrError = loadFromJson(*jsonOrError);
   ASSERT_TRUE(planOrError.ok());
 
-  auto plan = *planOrError;
+  const auto& plan = *planOrError;
 
   auto textResult = parseBinaryPlan(plan);
   auto textSymbols = textResult.getSymbolTable().getSymbols();
@@ -89,10 +89,10 @@ TEST_P(RoundTripBinaryToTextFixture, RoundTrip) {
   textResult.addErrors(errorListener.getErrorMessages());
 
   ASSERT_THAT(textResult, ParsesOk())
-      << std::endl
-      << "Initial result:" << std::endl
-      << addLineNumbers(outputText) << std::endl
-      << textResult.getSymbolTable().toDebugString() << std::endl;
+      << '\n'
+      << "Initial result:" << '\n'
+      << addLineNumbers(outputText) << '\n'
+      << textResult.getSymbolTable().toDebugString() << '\n';
 
   auto stream = loadTextString(outputText);
   auto result = parseStream(&stream);
@@ -108,10 +108,10 @@ TEST_P(RoundTripBinaryToTextFixture, RoundTrip) {
           AsBinaryPlan(IgnoringFields(
               {"substrait.proto.RelCommon.Emit.output_mapping"},
               EqualsProto(normalizedPlan)))))
-      << std::endl
-      << "Intermediate result:" << std::endl
-      << addLineNumbers(outputText) << std::endl
-      << result.getSymbolTable().toDebugString() << std::endl;
+      << '\n'
+      << "Intermediate result:" << '\n'
+      << addLineNumbers(outputText) << '\n'
+      << result.getSymbolTable().toDebugString() << '\n';
 }
 
 INSTANTIATE_TEST_SUITE_P(

src/substrait/textplan/tests/SymbolTableTest.cpp

@@ -102,7 +102,7 @@ TEST_F(SymbolTableTest, LocationsUnchangedAfterCopy) {
   auto* ptr3 = &plan.extensions(0);
 
   const SymbolTable& table2 = table;
-  auto symbols = table2.getSymbols();
+  const auto& symbols = table2.getSymbols();
   ASSERT_THAT(
       symbolNames(symbols),
       ::testing::ElementsAre("symbol1", "symbol2", "symbol3"));