Merge pull request #2514 from sensei-hacker/fix-transpiler-validation

Fix transpiler validation and improve error messages

Author: sensei-hacker

js/transpiler/transpiler/analyzer.js

@@ -197,8 +197,14 @@ class SemanticAnalyzer {
       // Variable assignment - allowed for var variables
       // (let reassignment already caught above)
     } else if (!this.isValidWritableProperty(stmt.target)) {
-      // Use original target (with inav. prefix) for writability check
-      this.addError(`Cannot assign to '${stmt.target}'. Not a valid INAV writable property.`, line);
+      // Check if it's an intermediate object and provide helpful error
+      const betterError = this.getImprovedWritabilityError(stmt.target, line);
+      if (betterError) {
+        this.addError(betterError, line);
+      } else {
+        // Use original target (with inav. prefix) for writability check
+        this.addError(`Cannot assign to '${stmt.target}'. Not a valid INAV writable property.`, line);
+      }
     }
 
     // Check if value references are valid
@@ -368,7 +374,111 @@ class SemanticAnalyzer {
   extractGvarIndex(gvarStr) {
     return this.propertyAccessChecker.extractGvarIndex(gvarStr);
   }
-  
+
+  /**
+   * Generate improved error message for invalid writable property assignments
+   * Detects intermediate objects and suggests correct nested properties
+   * @param {string} target - Property path (e.g., "inav.override.flightAxis.yaw")
+   * @param {number} line - Line number for error reporting
+   * @returns {string|null} Improved error message or null if no improvement available
+   */
+  getImprovedWritabilityError(target, line) {
+    // Strip 'inav.' prefix if present
+    const normalizedTarget = target.startsWith('inav.') ? target.substring(5) : target;
+    const parts = normalizedTarget.split('.');
+
+    // Only applies to override namespace for now
+    if (parts[0] !== 'override') {
+      return null;
+    }
+
+    // Check if trying to assign to a 3-level intermediate object
+    // E.g., override.flightAxis.yaw (should be override.flightAxis.yaw.angle or .rate)
+    if (parts.length === 3) {
+      const overrideDef = this.getOverrideDefinition(parts[1], parts[2]);
+
+      if (overrideDef && overrideDef.type === 'object' && overrideDef.properties) {
+        const availableProps = Object.keys(overrideDef.properties);
+        const suggestions = availableProps.map(p => `inav.override.${parts[1]}.${parts[2]}.${p}`).join(', ');
+        return `Cannot assign to '${target}' - it's an object, not a property. Available properties: ${suggestions}`;
+      }
+    }
+
+    // Check if trying to assign to a 2-level intermediate object
+    // E.g., override.vtx (should be override.vtx.power, etc.)
+    // or override.flightAxis (should be override.flightAxis.roll.angle, etc.)
+    if (parts.length === 2) {
+      const categoryDef = this.getOverrideCategoryDefinition(parts[1]);
+
+      if (categoryDef && categoryDef.type === 'object' && categoryDef.properties) {
+        const propKeys = Object.keys(categoryDef.properties);
+
+        // Check if properties are simple (like vtx.power) or nested (like flightAxis.roll.angle)
+        const firstProp = categoryDef.properties[propKeys[0]];
+
+        if (firstProp && firstProp.type === 'object' && firstProp.properties) {
+          // Deeply nested (like flightAxis.roll.angle)
+          const nestedPropKeys = Object.keys(firstProp.properties);
+          const suggestions = propKeys.slice(0, 2).flatMap(p => {
+            const nested = categoryDef.properties[p];
+            if (nested && nested.properties) {
+              const nestedKeys = Object.keys(nested.properties);
+              if (nestedKeys.length > 0) {
+                const firstNestedProp = nestedKeys[0];
+                return [`inav.override.${parts[1]}.${p}.${firstNestedProp}`];
+              }
+            }
+            return [];
+          }).join(', ');
+          return `Cannot assign to '${target}' - it's an object, not a property. Examples: ${suggestions}, ...`;
+        } else {
+          // Simple properties (like vtx.power, vtx.band, vtx.channel)
+          const suggestions = propKeys.map(p => `inav.override.${parts[1]}.${p}`).join(', ');
+          return `Cannot assign to '${target}' - it's an object, not a property. Available properties: ${suggestions}`;
+        }
+      }
+    }
+
+    return null;
+  }
+
+  /**
+   * Get override definition for a specific property
+   * @private
+   */
+  getOverrideDefinition(category, property) {
+    try {
+      // Access raw API definitions, not processed structure
+      const overrideDefs = apiDefinitions.override;
+      if (!overrideDefs) return null;
+
+      // For nested objects like flightAxis, check if the property itself has properties
+      if (overrideDefs[category] && overrideDefs[category].properties) {
+        return overrideDefs[category].properties[property];
+      }
+
+      return null;
+    } catch (error) {
+      return null;
+    }
+  }
+
+  /**
+   * Get override category definition
+   * @private
+   */
+  getOverrideCategoryDefinition(category) {
+    try {
+      // Access raw API definitions, not processed structure
+      const overrideDefs = apiDefinitions.override;
+      if (!overrideDefs) return null;
+
+      return overrideDefs[category];
+    } catch (error) {
+      return null;
+    }
+  }
+
   /**
    * Check for common unsupported JavaScript features
    */

js/transpiler/transpiler/index.js

@@ -130,6 +130,17 @@ class Transpiler {
       // Adjust line numbers if import was auto-added
       const adjustedWarnings = this.adjustLineNumbers(allWarnings, lineOffset);
 
+      // Categorize warnings to check for errors
+      const categorized = this.categorizeWarnings(adjustedWarnings);
+
+      // If there are parser errors (type='error'), fail the transpilation
+      if (categorized.errors && categorized.errors.length > 0) {
+        const errorMessages = categorized.errors.map(e =>
+          `  - ${e.message}${e.line ? ` (line ${e.line})` : ''}`
+        ).join('\n');
+        throw new Error(`Parse errors:\n${errorMessages}`);
+      }
+
       // Get gvar allocation summary
       const gvarSummary = this.analyzer.variableHandler ?
         this.analyzer.variableHandler.getAllocationSummary() :
@@ -142,7 +153,7 @@ class Transpiler {
         success: true,
         commands,
         logicConditionCount: this.codegen.lcIndex,
-        warnings: this.categorizeWarnings(adjustedWarnings),
+        warnings: categorized,
         optimizations: this.optimizer.getStats(),
         gvarUsage: gvarSummary,
         variableMap,

js/transpiler/transpiler/parser.js

@@ -361,9 +361,46 @@ class JavaScriptParser {
       }
     }
 
+    // Unrecognized function call - generate error instead of silently dropping
+    const calleeName = this.extractCalleeNameForError(expr.callee);
+    const line = loc ? loc.start.line : 0;
+    this.addWarning('error', `Cannot call '${calleeName}' as a function. Not a valid INAV function.`, line);
+
     return null;
   }
 
+  /**
+   * Extract callee name for error messages
+   * @private
+   */
+  extractCalleeNameForError(callee) {
+    if (callee.type === 'Identifier') {
+      return callee.name;
+    }
+    if (callee.type === 'MemberExpression') {
+      // Try to reconstruct the full path
+      const parts = [];
+      let current = callee;
+
+      while (current) {
+        if (current.type === 'MemberExpression') {
+          if (current.property) {
+            parts.unshift(current.property.name || current.property.value);
+          }
+          current = current.object;
+        } else if (current.type === 'Identifier') {
+          parts.unshift(current.name);
+          break;
+        } else {
+          break;
+        }
+      }
+
+      return parts.join('.');
+    }
+    return '<unknown>';
+  }
+
   /**
    * Transform helper functions (edge, sticky, delay, timer, whenChanged)
    */

js/transpiler/transpiler/property_access_checker.js

@@ -9,13 +9,15 @@
 
 'use strict';
 
+import apiDefinitions from '../api/definitions/index.js';
+
 /**
  * Helper class for checking property access validity
  */
 class PropertyAccessChecker {
   /**
    * @param {Object} context - Context object containing dependencies
-   * @param {Object} context.inavAPI - INAV API definitions
+   * @param {Object} context.inavAPI - INAV API definitions (processed structure)
    * @param {Function} context.getVariableHandler - Getter for variable handler instance
    * @param {Function} context.addError - Function to add errors
    * @param {Function} context.addWarning - Function to add warnings
@@ -184,10 +186,18 @@ class PropertyAccessChecker {
 
     const apiObj = this.inavAPI[apiCategory];
 
-    // For category-only access (e.g., "flight"), warn that it needs a property
+    // For category-only access (e.g., "flight"), error that it needs a property
     if (parts.length === startIndex) {
       if (apiObj.properties.length > 0 || Object.keys(apiObj.nested).length > 0) {
-        this.addWarning('incomplete-access', `'inav.${propPath}' needs a property. Did you mean to access a specific property?`, line);
+        // Build helpful error message with available properties
+        const availableProps = [
+          ...apiObj.properties.slice(0, 3).map(p => `inav.${apiCategory}.${p}`),
+          ...Object.keys(apiObj.nested).slice(0, 2).map(p => `inav.${apiCategory}.${p}.*`)
+        ].join(', ');
+        this.addError(
+          `Cannot use 'inav.${propPath}' - it's an object, not a property. Examples: ${availableProps}`,
+          line
+        );
       }
       return;
     }
@@ -203,12 +213,48 @@ class PropertyAccessChecker {
 
       // Check if it's a nested object
       if (apiObj.nested[propertyPart]) {
+        // Must have a nested property - can't use intermediate object directly
+        if (parts.length === startIndex + 1) {
+          // Accessing intermediate object without going deeper
+          const nestedProps = apiObj.nested[propertyPart];
+          const suggestions = nestedProps.slice(0, 3).map(p => `inav.${apiCategory}.${propertyPart}.${p}`).join(', ');
+          this.addError(
+            `Cannot use 'inav.${propPath}' - it's an object, not a property. ` +
+            `Available properties: ${suggestions}${nestedProps.length > 3 ? ', ...' : ''}`,
+            line
+          );
+          return;
+        }
+
         // Check nested property if present
         if (parts.length > startIndex + 1) {
           const nestedPart = parts[startIndex + 1];
           const nestedProps = apiObj.nested[propertyPart];
+
+          // Check if nested part is ALSO an object (3-level nesting like override.flightAxis.yaw)
+          // Need to check the API definition to see if this is a leaf or another object
           if (!nestedProps.includes(nestedPart)) {
             this.addError(`Unknown property '${nestedPart}' in 'inav.${propPath}'. Available: ${nestedProps.join(', ')}`, line);
+            return;
+          }
+
+          // Check if we stopped at a nested object (e.g., override.flightAxis.yaw instead of override.flightAxis.yaw.angle)
+          if (parts.length === startIndex + 2) {
+            // Need to check if nestedPart is itself an object
+            // This requires checking the actual API definition structure
+            const isNestedObject = this.isPropertyAnObject(apiCategory, propertyPart, nestedPart);
+            if (isNestedObject) {
+              const deeperProps = this.getNestedObjectProperties(apiCategory, propertyPart, nestedPart);
+              if (deeperProps && deeperProps.length > 0) {
+                const suggestions = deeperProps.slice(0, 3).map(p => `inav.${apiCategory}.${propertyPart}.${nestedPart}.${p}`).join(', ');
+                this.addError(
+                  `Cannot use 'inav.${propPath}' - it's an object, not a property. ` +
+                  `Available properties: ${suggestions}${deeperProps.length > 3 ? ', ...' : ''}`,
+                  line
+                );
+                return;
+              }
+            }
           }
         }
         return;
@@ -254,7 +300,7 @@ class PropertyAccessChecker {
    */
   checkWritableOverride(inavPath) {
     const parts = inavPath.split('.');
-    // parts = ['override', 'throttle'] or ['override', 'vtx', 'power']
+    // parts = ['override', 'throttle'] or ['override', 'vtx', 'power'] or ['override', 'flightAxis', 'yaw', 'angle']
 
     if (parts.length < 2) {
       return false;
@@ -270,9 +316,29 @@ class PropertyAccessChecker {
       return true;
     }
 
-    // Check nested properties (e.g., override.vtx.power)
+    // Check nested properties (e.g., override.vtx.power or override.flightAxis.yaw.angle)
     if (parts.length >= 3 && apiObj.nested && apiObj.nested[parts[1]]) {
-      return apiObj.nested[parts[1]].includes(parts[2]);
+      // Verify the property exists in the nested list
+      if (!apiObj.nested[parts[1]].includes(parts[2])) {
+        return false;
+      }
+
+      // For 3-level paths, need to check if parts[2] is itself an object (intermediate)
+      // If it has deeper nesting (like flightAxis.yaw.angle), the 3-level path is incomplete
+      if (parts.length === 3) {
+        // Check if this property is an intermediate object using the helper method
+        const isObject = this.isPropertyAnObject('override', parts[1], parts[2]);
+        if (isObject) {
+          // It's an intermediate object - need to go deeper (e.g., yaw.angle or yaw.rate)
+          return false;
+        }
+        // It's a leaf property - writable
+        return true;
+      }
+
+      // For 4+ level paths (e.g., override.flightAxis.yaw.angle), verify all levels exist
+      // This is handled by the broader validation in checkApiPropertyAccess
+      return true;
     }
 
     return false;
@@ -286,6 +352,60 @@ class PropertyAccessChecker {
     const match = gvarStr.match(/gvar\[(\d+)\]/);
     return match ? parseInt(match[1]) : -1;
   }
+
+  /**
+   * Check if a property is itself an object (not a leaf value)
+   * Used to detect 3-level nested objects like override.flightAxis.yaw
+   * @param {string} category - API category (e.g., 'override')
+   * @param {string} parentProp - Parent property (e.g., 'flightAxis')
+   * @param {string} childProp - Child property (e.g., 'yaw')
+   * @returns {boolean} True if childProp is an object with more properties
+   * @private
+   */
+  isPropertyAnObject(category, parentProp, childProp) {
+    try {
+      // Use raw API definitions to preserve nested structure
+      const categoryDef = apiDefinitions[category];
+      if (!categoryDef) return false;
+
+      const parentDef = categoryDef[parentProp];
+      if (!parentDef || !parentDef.properties) return false;
+
+      const childDef = parentDef.properties[childProp];
+      if (!childDef) return false;
+
+      // Simple check: typeof object with properties
+      return childDef.type === 'object' && childDef.properties && Object.keys(childDef.properties).length > 0;
+    } catch (error) {
+      return false;
+    }
+  }
+
+  /**
+   * Get properties of a nested object
+   * @param {string} category - API category (e.g., 'override')
+   * @param {string} parentProp - Parent property (e.g., 'flightAxis')
+   * @param {string} childProp - Child property (e.g., 'yaw')
+   * @returns {string[]|null} Array of property names or null
+   * @private
+   */
+  getNestedObjectProperties(category, parentProp, childProp) {
+    try {
+      // Use raw API definitions
+      const categoryDef = apiDefinitions[category];
+      if (!categoryDef) return null;
+
+      const parentDef = categoryDef[parentProp];
+      if (!parentDef || !parentDef.properties) return null;
+
+      const childDef = parentDef.properties[childProp];
+      if (!childDef || !childDef.properties) return null;
+
+      return Object.keys(childDef.properties);
+    } catch (error) {
+      return null;
+    }
+  }
 }
 
 export { PropertyAccessChecker };