Add check to disallow field in on_insert() (#920)

* Add check to disallow field in on_insert()

* revert some clang-tidy changes to avoid diffs

* another attempt to fix formatting issues

* removed unnecessary line of code

* responding to PR comments

production/tools/gaia_translate/src/main.cpp

@@ -1629,10 +1629,10 @@ void update_expression_explicit_path_data(
             {
                 string first_component = get_table_from_expression(data.path_components.front());
                 const auto tag_iterator = data.tag_table_map.find(first_component);
-                if (tag_iterator != data.tag_table_map.end() &&
-                    (tag_iterator->second != tag_iterator->first ||
-                     explicit_path_data_iterator != g_expression_explicit_path_data.end() ||
-                     can_path_be_optimized(first_component, expression_explicit_path_data_iterator.second)))
+                if (tag_iterator != data.tag_table_map.end()
+                    && (tag_iterator->second != tag_iterator->first
+                        || explicit_path_data_iterator != g_expression_explicit_path_data.end()
+                        || can_path_be_optimized(first_component, expression_explicit_path_data_iterator.second)))
                 {
                     data.skip_implicit_path_generation = true;
                 }
@@ -2477,6 +2477,12 @@ class rule_match_handler_t : public MatchFinder::MatchCallback
                 string table, field, tag;
                 if (parse_attribute(table_iterator, table, field, tag))
                 {
+                    if (!field.empty())
+                    {
+                        gaiat::diag().emit(diag::err_illegal_field_reference);
+                        g_is_generation_error = true;
+                        return;
+                    }
                     g_insert_tables.insert(table);
                     if (!tag.empty())
                     {
@@ -2950,12 +2956,12 @@ class declarative_insert_handler_t : public MatchFinder::MatchCallback
             size_t argument_name_end_position = raw_argument_name.find(':');
             string argument_name = raw_argument_name.substr(0, argument_name_end_position);
             // Trim the argument name of whitespaces.
-            argument_name.erase(argument_name.begin(), find_if(argument_name.begin(), argument_name.end(), [](unsigned char ch)
-                                                               { return !isspace(ch); }));
-            argument_name.erase(find_if(argument_name.rbegin(), argument_name.rend(), [](unsigned char ch)
-                                        { return !isspace(ch); })
-                                    .base(),
-                                argument_name.end());
+            argument_name.erase(
+                argument_name.begin(),
+                find_if(argument_name.begin(), argument_name.end(), [](unsigned char ch) { return !isspace(ch); }));
+            argument_name.erase(
+                find_if(argument_name.rbegin(), argument_name.rend(), [](unsigned char ch) { return !isspace(ch); }).base(),
+                argument_name.end());
             insert_data.argument_map[argument_name] = argument->getSourceRange();
             insert_data.argument_replacement_map[argument->getSourceRange()] = m_rewriter.getRewrittenText(argument->getSourceRange());
         }
@@ -3544,8 +3550,8 @@ class gaiat_diagnostic_consumer_t : public clang::TextDiagnosticPrinter
         Rewriter& rewriter = *m_rewriter;
 
         // Always call the TextDiagnosticPrinter's EndSourceFile() method.
-        auto call_end_source_file = gaia::common::scope_guard::make_scope_guard([this]
-                                                                                { TextDiagnosticPrinter::EndSourceFile(); });
+        auto call_end_source_file = gaia::common::scope_guard::make_scope_guard(
+            [this] { TextDiagnosticPrinter::EndSourceFile(); });
 
         generate_rules(rewriter);
         if (g_is_generation_error)

production/tools/tests/gaiat/integration_test.ruleset

@@ -154,10 +154,6 @@ ruleset test12
 // are multi-anchor rules.
 ruleset test13
 {
-    on_insert(sensor.value, sensor.timestamp)
-    {
-    }
-
     on_insert(sensor, sensor)
     {
     }

production/tools/tests/gaiat/integration_test_expected_incorrect_table_name.ruleset

@@ -0,0 +1,12 @@
+// RUN: not %gaiat %s -- 2>&1| FileCheck  %s
+
+ruleset test109
+{
+    on_insert(INC:incubator.max_temp)
+    {
+        incubator.min_temp = incubator.max_temp;
+    }
+}
+
+// GAIAPLAT-1396
+// CHECK: A field may not be referenced in the 'on_insert' statement. The 'on_insert' attribute only accepts a table.

production/tools/tests/gaiat/tags_tests.ruleset

@@ -136,16 +136,6 @@ ruleset test14
     }
 }
 
-ruleset test15
-{
-    // NOTE: the ".min_temp" field didn't cause this rule to fire, but
-    // may serve as documentation for the rule's intended use.
-    on_insert(I:incubator.min_temp)
-    {
-        use_float(I.min_temp);
-    }
-}
-
 // GAIAPLAT-829 (fixed)
 ruleset test16
 {

third_party/production/TranslationEngineLLVM/clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -9591,6 +9591,8 @@ def err_multi_anchor_tables : Error<
   "A rule may not specify multiple tables or active fields from different tables "
   "in 'on_' attributes (on_insert, on_change, or on_update). "
   "Ensure that all your active field references belong to the same table.">;
+def err_illegal_field_reference : Error<
+  "A field may not be referenced in the 'on_insert' statement. The 'on_insert' attribute only accepts a table.">;
 def err_multiple_shortest_paths : Error<
   "Multiple shortest paths exist between tables '%0' and '%1'. "
   "Explicitly specify the navigation path between these two tables to resolve the ambiguity.">;