Allow struct to contain member with same name (#133)

According to IDL spec, structs members consist of declarators rather than scoped names. So it's okay for them to have the same name because the struct member identifiers do not exist outside of the struct scope. This also mirrors C behavior: https://stackoverflow.com/questions/15397502/in-c-can-struct-members-use-the-same-name-as-a-type Fixes: FG-6541 and #132 --------- Co-authored-by: Jacob Bandes-Storch <[email protected]>
foxglove · Feb 13, 2024 · 586ee40 · 586ee40
1 parent e68ca22
commit 586ee40
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 6 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -1,7 +1,7 @@
 // -*- jsonc -*-
 {
   "editor.codeActionsOnSave": {
-    "source.fixAll.eslint": true
+    "source.fixAll.eslint": "explicit"
   },
   "editor.defaultFormatter": "esbenp.prettier-vscode",
   "editor.detectIndentation": false,

diff --git a/packages/omgidl-parser/src/IDLNodes/IDLNode.ts b/packages/omgidl-parser/src/IDLNodes/IDLNode.ts
@@ -92,18 +92,18 @@ function resolveScopedOrLocalNodeReference({
   // If using local un-scoped identifier, it will not be found in the definitions map
   // In this case we try by building up the namespace prefix until we find a match
   let referencedNode = undefined;
-  const namespacePrefixes = [...scopeOfUsage];
-  const currPrefix: string[] = [];
+  // start with most specific scope and work upwards
+  const currPrefix: string[] = [...scopeOfUsage];
   for (;;) {
     const identifierToTry = toScopedIdentifier([...currPrefix, usedIdentifier]);
     referencedNode = definitionMap.get(identifierToTry);
     if (referencedNode != undefined) {
       break;
     }
-    if (namespacePrefixes.length === 0) {
+    if (currPrefix.length === 0) {
       break;
     }
-    currPrefix.push(namespacePrefixes.shift()!);
+    currPrefix.pop();
   }
 
   return referencedNode;

diff --git a/packages/omgidl-parser/src/parseIDL.test.ts b/packages/omgidl-parser/src/parseIDL.test.ts
@@ -1,4 +1,4 @@
-import { parseIDL as parse } from "./parseIDL";
+import { parseIDL as parse, parseIDL } from "./parseIDL";
 
 describe("omgidl parser tests", () => {
   it("parses a struct", () => {
@@ -2597,6 +2597,28 @@ module rosidl_parser {
       },
     ]);
   });
+  it("can parse struct with member of the same name", () => {
+    const msgDef = `
+    struct ColorSettings {
+        uint8 ColorSettings;
+    };
+      `;
+
+    const ast = parseIDL(msgDef);
+    expect(ast).toEqual([
+      {
+        name: "ColorSettings",
+        aggregatedKind: "struct",
+        definitions: [
+          {
+            name: "ColorSettings",
+            isComplex: false,
+            type: "uint8",
+          },
+        ],
+      },
+    ]);
+  });
   // **************** Not supported in our implementation yet
   it("cannot compose variable size arrays (no serialization support)", () => {
     const msgDef = `

diff --git a/packages/omgidl-parser/src/parseIDLToAST.test.ts b/packages/omgidl-parser/src/parseIDLToAST.test.ts
@@ -1775,7 +1775,29 @@ module idl_parser {
       },
     ]);
   });
+  it("can parse struct with member of the same name", () => {
+    const msgDef = `
+    struct ColorSettings {
+        uint8 ColorSettings;
+    };
+      `;
 
+    const ast = parseIDLToAST(msgDef);
+    expect(ast).toEqual([
+      {
+        name: "ColorSettings",
+        declarator: "struct",
+        definitions: [
+          {
+            name: "ColorSettings",
+            isComplex: false,
+            declarator: "struct-member",
+            type: "uint8",
+          },
+        ],
+      },
+    ]);
+  });
   /****************  Not supported by IDL (as far as I can tell) */
   it("cannot parse constants that reference other constants", () => {
     const msgDef = `