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

This PR applies more fixes found by `clang-tidy`. Curiously, they only
show up with #148, which consumes `abseil-cpp` differently.

Signed-off-by: Ingo Müller <ingomueller@google.com>

Author: ingomueller-net

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/SubstraitPlanRelationVisitor.cpp

@@ -35,7 +35,7 @@ const std::string kSortDirectionPrefix = "sortdirection";
 
 const std::string kIntermediateNodeName = "intermediate";
 
-enum RelationFilterBehavior {
+enum RelationFilterBehavior : std::int8_t {
   kDefault = 0,
   kBestEffort = 1,
   kPostJoin = 2,

src/substrait/textplan/parser/SubstraitPlanSubqueryRelationVisitor.cpp

@@ -33,7 +33,7 @@ const std::string kAggregationInvocationPrefix = "aggregationinvocation";
 const std::string kJoinTypePrefix = "jointype";
 const std::string kSortDirectionPrefix = "sortdirection";
 
-enum RelationFilterBehavior {
+enum RelationFilterBehavior : std::int8_t {
   kDefault = 0,
   kBestEffort = 1,
   kPostJoin = 2,

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;
   }