From bacc82deba9d676d88efd0da6fa45da96e0cd856 Mon Sep 17 00:00:00 2001 From: nejimakibird <23066051+nejimakibird@users.noreply.github.com> Date: Fri, 19 Jun 2026 19:21:10 +0900 Subject: [PATCH] fix: detect duplicate mapping target members --- main.js | 92 +++++++++++++-- src/core/current-file-diagnostics.ts | 108 ++++++++++++++++-- src/types/warnings.ts | 3 +- test/domains-model.test.mjs | 163 +++++++++++++++++++++++++++ 4 files changed, 351 insertions(+), 15 deletions(-) diff --git a/main.js b/main.js index efd18da..04dbc19 100644 --- a/main.js +++ b/main.js @@ -1567,7 +1567,8 @@ function buildRuleDiagnostics(model, index) { } function buildMappingDiagnostics(model, index) { const diagnostics = []; - const targetRefs = /* @__PURE__ */ new Set(); + const mappingRows = /* @__PURE__ */ new Set(); + const targetMemberRows = /* @__PURE__ */ new Map(); for (const scope of model.scope) { diagnostics.push( ...buildReferenceWarnings(model.path, "Scope", scope.ref, index, "unresolved scope reference") @@ -1578,13 +1579,39 @@ function buildMappingDiagnostics(model, index) { const sourceRef = row.sourceRef?.trim(); const transform = row.transform?.trim(); const required = row.required?.trim(); + const rule = row.rule?.trim(); if (!targetRef) { diagnostics.push(createSectionWarning(model.path, "Mappings", "target_ref is empty")); - } else { - if (targetRefs.has(targetRef)) { - diagnostics.push(createSectionWarning(model.path, "Mappings", `duplicate target_ref "${targetRef}"`)); + } + if (sourceRef && targetRef) { + const duplicateKey = buildMappingRowDuplicateKey(sourceRef, targetRef, transform, rule); + if (mappingRows.has(duplicateKey)) { + diagnostics.push( + createSectionWarning( + model.path, + "Mappings", + `duplicate mapping row "${formatMappingReferenceForMessage(sourceRef)} -> ${formatMappingReferenceForMessage(targetRef)}"` + ) + ); + } + mappingRows.add(duplicateKey); + } + if (targetRef) { + const targetMember = getMappingTargetMemberReference(targetRef); + if (targetMember) { + const rowKey = buildMappingRowDuplicateKey(sourceRef, targetRef, transform, rule); + const existing = targetMemberRows.get(targetMember.key) ?? { + display: targetMember.display, + rowKeys: /* @__PURE__ */ new Set(), + warned: false + }; + existing.rowKeys.add(rowKey); + if (!existing.warned && existing.rowKeys.size > 1) { + diagnostics.push(createDuplicateMappingTargetMemberWarning(model.path, targetMember.display)); + existing.warned = true; + } + targetMemberRows.set(targetMember.key, existing); } - targetRefs.add(targetRef); } if (!sourceRef && !transform) { diagnostics.push(createSectionWarning(model.path, "Mappings", "source_ref is empty and transform is also empty")); @@ -1599,9 +1626,9 @@ function buildMappingDiagnostics(model, index) { ...buildReferenceWarnings(model.path, "Mappings", targetRef, index, "unresolved mapping target_ref") ); } - if (row.rule?.trim()) { + if (rule) { diagnostics.push( - ...buildReferenceWarnings(model.path, "Mappings", row.rule, index, "unresolved mapping rule reference") + ...buildReferenceWarnings(model.path, "Mappings", rule, index, "unresolved mapping rule reference") ); } if (required && required !== "Y" && required !== "N") { @@ -1610,6 +1637,55 @@ function buildMappingDiagnostics(model, index) { } return diagnostics; } +function buildMappingRowDuplicateKey(sourceRef, targetRef, transform, rule) { + return [ + normalizeMappingReferenceKey(sourceRef), + normalizeMappingReferenceKey(targetRef), + normalizeMappingFreeTextKey(transform), + normalizeMappingReferenceKey(rule) + ].join(" "); +} +function getMappingTargetMemberReference(reference) { + const qualified = parseQualifiedRef(reference); + if (!qualified?.hasMemberRef || !qualified.memberRef) { + return null; + } + const display = formatMappingReferenceForMessage(reference); + return { + key: normalizeMappingReferenceKey(reference), + display + }; +} +function createDuplicateMappingTargetMemberWarning(path2, display) { + return { + code: "duplicate-mapping-target-member", + message: `mapping target member "${display}" is mapped from multiple sources.`, + severity: "warning", + path: path2, + field: "Mappings", + context: { section: "Mappings", reference: display } + }; +} +function normalizeMappingReferenceKey(reference) { + return formatMappingReferenceForMessage(reference).toLowerCase(); +} +function normalizeMappingFreeTextKey(value) { + return value?.trim().replace(/\s+/g, " ").toLowerCase() ?? ""; +} +function formatMappingReferenceForMessage(reference) { + const trimmed = reference?.trim(); + if (!trimmed) { + return ""; + } + const qualified = parseQualifiedRef(trimmed); + if (qualified) { + const parsedBase = parseReferenceValue(qualified.baseRefRaw); + const base = parsedBase?.target?.trim() || qualified.baseRefRaw.trim(); + return qualified.memberRef ? `${base}.${qualified.memberRef}` : base; + } + const parsed = parseReferenceValue(trimmed); + return parsed?.target?.trim() || trimmed; +} function buildAppProcessDiagnostics(model, index) { const diagnostics = []; const inputIds = /* @__PURE__ */ new Set(); @@ -2377,6 +2453,8 @@ function localizeDiagnosticMessage(message, language) { [/^Fields table mixes standard and file layout columns; parsed as file_layout$/, "Fields \u30C6\u30FC\u30D6\u30EB\u306B standard \u5F62\u5F0F\u3068 file_layout \u5F62\u5F0F\u306E\u5217\u304C\u6DF7\u5728\u3057\u3066\u3044\u307E\u3059\u3002file_layout \u3068\u3057\u3066\u89E3\u6790\u3057\u307E\u3057\u305F\u3002"], [/^duplicate field name "([^"]+)"$/, (_match, name) => `\u30D5\u30A3\u30FC\u30EB\u30C9\u540D "${name}" \u304C\u91CD\u8907\u3057\u3066\u3044\u307E\u3059\u3002`], [/^duplicate field id "([^"]+)"$/, (_match, id) => `Fields.id "${id}" \u304C\u91CD\u8907\u3057\u3066\u3044\u307E\u3059\u3002`], + [/^duplicate mapping row "([^"]+)"$/, (_match, value) => `mapping row "${value}" \u304C\u91CD\u8907\u3057\u3066\u3044\u307E\u3059\u3002`], + [/^mapping target member "([^"]+)" is mapped from multiple sources\.$/, (_match, value) => `mapping target member "${value}" \u304C\u8907\u6570\u306E source_ref \u304B\u3089\u5BFE\u5FDC\u4ED8\u3051\u3089\u308C\u3066\u3044\u307E\u3059\u3002`], [/^duplicate (.+) "([^"]+)"$/, (_match, target, value) => `${target} "${value}" \u304C\u91CD\u8907\u3057\u3066\u3044\u307E\u3059\u3002`], [/^duplicate (ER relation id): (.+)$/, (_match, target, value) => `${target}: ${value} \u304C\u91CD\u8907\u3057\u3066\u3044\u307E\u3059\u3002`], [/^duplicate id detected: "([^"]+)"$/, (_match, id) => `id "${id}" \u304C\u91CD\u8907\u3057\u3066\u3044\u307E\u3059\u3002`], diff --git a/src/core/current-file-diagnostics.ts b/src/core/current-file-diagnostics.ts index 77fdf40..e5544bb 100644 --- a/src/core/current-file-diagnostics.ts +++ b/src/core/current-file-diagnostics.ts @@ -248,7 +248,8 @@ function buildMappingDiagnostics( index: ModelingVaultIndex ): ValidationWarning[] { const diagnostics: ValidationWarning[] = []; - const targetRefs = new Set(); + const mappingRows = new Set(); + const targetMemberRows = new Map; warned: boolean }>(); for (const scope of model.scope) { diagnostics.push( @@ -261,14 +262,42 @@ function buildMappingDiagnostics( const sourceRef = row.sourceRef?.trim(); const transform = row.transform?.trim(); const required = row.required?.trim(); + const rule = row.rule?.trim(); if (!targetRef) { diagnostics.push(createSectionWarning(model.path, "Mappings", "target_ref is empty")); - } else { - if (targetRefs.has(targetRef)) { - diagnostics.push(createSectionWarning(model.path, "Mappings", `duplicate target_ref "${targetRef}"`)); + } + + if (sourceRef && targetRef) { + const duplicateKey = buildMappingRowDuplicateKey(sourceRef, targetRef, transform, rule); + if (mappingRows.has(duplicateKey)) { + diagnostics.push( + createSectionWarning( + model.path, + "Mappings", + `duplicate mapping row "${formatMappingReferenceForMessage(sourceRef)} -> ${formatMappingReferenceForMessage(targetRef)}"` + ) + ); + } + mappingRows.add(duplicateKey); + } + + if (targetRef) { + const targetMember = getMappingTargetMemberReference(targetRef); + if (targetMember) { + const rowKey = buildMappingRowDuplicateKey(sourceRef, targetRef, transform, rule); + const existing = targetMemberRows.get(targetMember.key) ?? { + display: targetMember.display, + rowKeys: new Set(), + warned: false + }; + existing.rowKeys.add(rowKey); + if (!existing.warned && existing.rowKeys.size > 1) { + diagnostics.push(createDuplicateMappingTargetMemberWarning(model.path, targetMember.display)); + existing.warned = true; + } + targetMemberRows.set(targetMember.key, existing); } - targetRefs.add(targetRef); } if (!sourceRef && !transform) { @@ -285,9 +314,9 @@ function buildMappingDiagnostics( ...buildReferenceWarnings(model.path, "Mappings", targetRef, index, "unresolved mapping target_ref") ); } - if (row.rule?.trim()) { + if (rule) { diagnostics.push( - ...buildReferenceWarnings(model.path, "Mappings", row.rule, index, "unresolved mapping rule reference") + ...buildReferenceWarnings(model.path, "Mappings", rule, index, "unresolved mapping rule reference") ); } if (required && required !== "Y" && required !== "N") { @@ -298,6 +327,69 @@ function buildMappingDiagnostics( return diagnostics; } +function buildMappingRowDuplicateKey( + sourceRef: string | undefined, + targetRef: string | undefined, + transform: string | undefined, + rule: string | undefined +): string { + return [ + normalizeMappingReferenceKey(sourceRef), + normalizeMappingReferenceKey(targetRef), + normalizeMappingFreeTextKey(transform), + normalizeMappingReferenceKey(rule) + ].join("\t"); +} + +function getMappingTargetMemberReference(reference: string): { key: string; display: string } | null { + const qualified = parseQualifiedRef(reference); + if (!qualified?.hasMemberRef || !qualified.memberRef) { + return null; + } + + const display = formatMappingReferenceForMessage(reference); + return { + key: normalizeMappingReferenceKey(reference), + display + }; +} + +function createDuplicateMappingTargetMemberWarning(path: string, display: string): ValidationWarning { + return { + code: "duplicate-mapping-target-member", + message: `mapping target member "${display}" is mapped from multiple sources.`, + severity: "warning", + path, + field: "Mappings", + context: { section: "Mappings", reference: display } + }; +} + +function normalizeMappingReferenceKey(reference: string | undefined): string { + return formatMappingReferenceForMessage(reference).toLowerCase(); +} + +function normalizeMappingFreeTextKey(value: string | undefined): string { + return value?.trim().replace(/\s+/g, " ").toLowerCase() ?? ""; +} + +function formatMappingReferenceForMessage(reference: string | undefined): string { + const trimmed = reference?.trim(); + if (!trimmed) { + return ""; + } + + const qualified = parseQualifiedRef(trimmed); + if (qualified) { + const parsedBase = parseReferenceValue(qualified.baseRefRaw); + const base = parsedBase?.target?.trim() || qualified.baseRefRaw.trim(); + return qualified.memberRef ? `${base}.${qualified.memberRef}` : base; + } + + const parsed = parseReferenceValue(trimmed); + return parsed?.target?.trim() || trimmed; +} + function buildAppProcessDiagnostics( model: AppProcessModel, index: ModelingVaultIndex @@ -1256,6 +1348,8 @@ export function localizeDiagnosticMessage(message: string, language?: string): s [/^Fields table mixes standard and file layout columns; parsed as file_layout$/, "Fields テーブルに standard 形式と file_layout 形式の列が混在しています。file_layout として解析しました。"], [/^duplicate field name "([^"]+)"$/, (_match, name) => `フィールド名 "${name}" が重複しています。`], [/^duplicate field id "([^"]+)"$/, (_match, id) => `Fields.id "${id}" が重複しています。`], + [/^duplicate mapping row "([^"]+)"$/, (_match, value) => `mapping row "${value}" が重複しています。`], + [/^mapping target member "([^"]+)" is mapped from multiple sources\.$/, (_match, value) => `mapping target member "${value}" が複数の source_ref から対応付けられています。`], [/^duplicate (.+) "([^"]+)"$/, (_match, target, value) => `${target} "${value}" が重複しています。`], [/^duplicate (ER relation id): (.+)$/, (_match, target, value) => `${target}: ${value} が重複しています。`], [/^duplicate id detected: "([^"]+)"$/, (_match, id) => `id "${id}" が重複しています。`], diff --git a/src/types/warnings.ts b/src/types/warnings.ts index 5906f8d..77f299f 100644 --- a/src/types/warnings.ts +++ b/src/types/warnings.ts @@ -29,7 +29,8 @@ export const VALIDATION_WARNING_CODES = [ "invalid-numeric-value", "legacy-class-relation-format", "legacy-class-relation-from-mismatch", - "class-relation-target-not-diagram-compatible" + "class-relation-target-not-diagram-compatible", + "duplicate-mapping-target-member" ] as const; export type ValidationWarningSeverity = diff --git a/test/domains-model.test.mjs b/test/domains-model.test.mjs index be370dd..32a9f3a 100644 --- a/test/domains-model.test.mjs +++ b/test/domains-model.test.mjs @@ -2184,6 +2184,169 @@ name: Menu actions ); }); + + +test("mapping diagnostics treat member references and mapping rows as duplicate keys", () => { + const index = buildVaultIndex([ + { path: "DATA-MAP-SOURCE.md", content: `--- +type: data_object +id: DATA-MAP-SOURCE +name: Source data +--- + +# Source data + +## Fields + +| name | label | type | length | required | path | ref | notes | +|---|---|---|---|---|---|---|---| +| issue | Issue | string | | N | | | | +| notes | Notes | string | | N | | | | +` }, + { path: "DATA-MAP-TARGET.md", content: `--- +type: data_object +id: DATA-MAP-TARGET +name: Target data +--- + +# Target data + +## Fields + +| name | label | type | length | required | path | ref | notes | +|---|---|---|---|---|---|---|---| +| status_id | Status | string | | N | | | | +| subject | Subject | string | | N | | | | +` }, + { path: "MAP-MEMBER-DUPLICATES.md", content: `--- +type: mapping +id: MAP-MEMBER-DUPLICATES +name: Member duplicates +source: [[DATA-MAP-SOURCE]] +target: [[DATA-MAP-TARGET]] +--- + +# Member duplicates + +## Mappings + +| source_ref | target_ref | transform | rule | required | notes | +|---|---|---|---|---|---| +| [[DATA-MAP-SOURCE]].issue | [[DATA-MAP-TARGET]].status_id | copy | | Y | field mapping | +| [[DATA-MAP-SOURCE]].notes | [[DATA-MAP-TARGET]].status_id | copy | | Y | same target allowed | +| [[DATA-MAP-SOURCE]].issue | [[DATA-MAP-TARGET]].subject | copy | | Y | different member target | +| [[DATA-MAP-SOURCE]].issue | [[DATA-MAP-TARGET]].subject | copy | | Y | duplicate row | +` } + ], { parseMode: "full" }); + + const model = index.modelsByFilePath["MAP-MEMBER-DUPLICATES.md"]; + assert.equal(model.fileType, "mapping"); + const diagnostics = buildCurrentObjectDiagnostics( + model, + index, + null, + index.warningsByFilePath["MAP-MEMBER-DUPLICATES.md"] ?? [] + ); + const duplicateMessages = diagnostics + .map((warning) => warning.message) + .filter((message) => message.includes("duplicate")); + const targetMemberWarnings = diagnostics.filter( + (warning) => warning.code === "duplicate-mapping-target-member" + ); + + assert.deepEqual(duplicateMessages, [ + 'duplicate mapping row "DATA-MAP-SOURCE.issue -> DATA-MAP-TARGET.subject"' + ]); + assert.deepEqual( + targetMemberWarnings.map((warning) => warning.message), + [ + 'mapping target member "DATA-MAP-TARGET.status_id" is mapped from multiple sources.' + ] + ); + assert.equal( + diagnostics.some((warning) => warning.message.includes("[[")), + false + ); + assert.equal( + localizeDiagnosticMessage(duplicateMessages[0], "ja"), + 'mapping row "DATA-MAP-SOURCE.issue -> DATA-MAP-TARGET.subject" が重複しています。' + ); + assert.equal( + localizeDiagnosticMessage(targetMemberWarnings[0].message, "ja"), + 'mapping target member "DATA-MAP-TARGET.status_id" が複数の source_ref から対応付けられています。' + ); +}); + +test("mapping diagnostics allow repeated target refs when mapping rows differ", () => { + const index = buildVaultIndex([ + { path: "DATA-MAP-SOURCE-A.md", content: `--- +type: data_object +id: DATA-MAP-SOURCE-A +name: Source A +--- + +# Source A +` }, + { path: "DATA-MAP-SOURCE-B.md", content: `--- +type: data_object +id: DATA-MAP-SOURCE-B +name: Source B +--- + +# Source B +` }, + { path: "ENT-MAP-TARGET.md", content: `--- +type: er_entity +id: ENT-MAP-TARGET +logical_name: Target entity +physical_name: target_entities +--- + +# Target entity +` }, + { path: "MAP-REPEATED-TARGET.md", content: `--- +type: mapping +id: MAP-REPEATED-TARGET +name: Repeated target +source: [[DATA-MAP-SOURCE-A]] +target: [[ENT-MAP-TARGET]] +--- + +# Repeated target + +## Mappings + +| source_ref | target_ref | transform | rule | required | notes | +|---|---|---|---|---|---| +| [[DATA-MAP-SOURCE-A]] | [[ENT-MAP-TARGET]] | copy | | Y | first source | +| [[DATA-MAP-SOURCE-B]] | [[ENT-MAP-TARGET]] | copy | | Y | second source | +| [[DATA-MAP-SOURCE-A]] | [[ENT-MAP-TARGET]] | normalize | | Y | different transform | +` } + ], { parseMode: "full" }); + + const model = index.modelsByFilePath["MAP-REPEATED-TARGET.md"]; + assert.equal(model.fileType, "mapping"); + const diagnostics = buildCurrentObjectDiagnostics( + model, + index, + null, + index.warningsByFilePath["MAP-REPEATED-TARGET.md"] ?? [] + ); + + assert.equal( + diagnostics.some((warning) => warning.message.includes("duplicate target_ref")), + false + ); + assert.equal( + diagnostics.some((warning) => warning.message.includes("duplicate mapping row")), + false + ); + assert.equal( + diagnostics.some((warning) => warning.code === "duplicate-mapping-target-member"), + false + ); +}); + test("DFD-local Domains parse without becoming DFD objects", () => { const { file, warnings } = parseDfd(dfdBody([ "| logistics | Logistics | department | | Local logistics |",