fix: review comment

SAP · Oct 15, 2024 · 8937e72 · 8937e72
1 parent b881d7b
commit 8937e72
Show file tree

Hide file tree

Showing 13 changed files with 45 additions and 45 deletions.
diff --git a/packages/binding/test/unit/services/completion/aggregation-binding.test.ts b/packages/binding/test/unit/services/completion/aggregation-binding.test.ts
@@ -39,7 +39,7 @@ describe("aggregation binding", () => {
       server: "off",
     },
     SplitAttributesOnFormat: true,
-    ReportNonUniqueIdsCrossViewFiles: true,
+    LimitUniqueIdDiagnostics: false,
   };
 
   beforeAll(async function () {

diff --git a/packages/binding/test/unit/services/completion/property-binding-info.test.ts b/packages/binding/test/unit/services/completion/property-binding-info.test.ts
@@ -44,7 +44,7 @@ describe("index", () => {
       server: "off",
     },
     SplitAttributesOnFormat: true,
-    ReportNonUniqueIdsCrossViewFiles: true,
+    LimitUniqueIdDiagnostics: false,
   };
 
   beforeAll(async function () {

diff --git a/packages/fe/test/unit/services/completion/providers/context-path.test.ts b/packages/fe/test/unit/services/completion/providers/context-path.test.ts
@@ -45,7 +45,7 @@ describe("contextPath attribute value completion", () => {
       server: "off",
     },
     SplitAttributesOnFormat: true,
-    ReportNonUniqueIdsCrossViewFiles: true,
+    LimitUniqueIdDiagnostics: false,
   };
 
   const annotationSnippetCDS = `

diff --git a/packages/fe/test/unit/services/completion/providers/filter-bar.test.ts b/packages/fe/test/unit/services/completion/providers/filter-bar.test.ts
@@ -43,7 +43,7 @@ describe("filterBar id attribute value completion", () => {
       server: "off",
     },
     SplitAttributesOnFormat: true,
-    ReportNonUniqueIdsCrossViewFiles: true,
+    LimitUniqueIdDiagnostics: false,
   };
 
   const annotationSnippetCDS = `

diff --git a/packages/fe/test/unit/services/completion/providers/meta-path.test.ts b/packages/fe/test/unit/services/completion/providers/meta-path.test.ts
@@ -48,7 +48,7 @@ describe("metaPath attribute value completion", () => {
       server: "off",
     },
     SplitAttributesOnFormat: true,
-    ReportNonUniqueIdsCrossViewFiles: true,
+    LimitUniqueIdDiagnostics: false,
   };
 
   const annotationSnippetCDS = `

diff --git a/packages/language-server/src/server.ts b/packages/language-server/src/server.ts
@@ -416,11 +416,11 @@ documents.onDidChangeContent(async (changeEvent): Promise<void> => {
     });
     documentsDiagnostics.set(document.uri, diagnostics);
     const settings = getConfigurationSettings();
-    const reportNonUniqueIds = settings.ReportNonUniqueIdsCrossViewFiles;
-    if (reportNonUniqueIds) {
-      await validateIdsOfOpenDocuments();
-    } else {
+    const limitUniqueIdsDiagReport = settings.LimitUniqueIdDiagnostics;
+    if (limitUniqueIdsDiagReport) {
       validateIdsOfOpenDocument(document, context);
+    } else {
+      await validateIdsOfOpenDocuments();
     }
   }
 });

diff --git a/packages/settings/api.d.ts b/packages/settings/api.d.ts
@@ -13,14 +13,14 @@ export type Settings = CodeAssistSettings &
   TraceSettings &
   LoggingSettings &
   WebServerSettings &
-  NonUniqueIdsSettings &
+  LimitUniqueIdDiagnostics &
   FormatterSettings;
 
 export interface WebServerSettings {
   SAPUI5WebServer?: string;
 }
-export interface NonUniqueIdsSettings {
-  ReportNonUniqueIdsCrossViewFiles: boolean;
+export interface LimitUniqueIdDiagnostics {
+  LimitUniqueIdDiagnostics: boolean;
 }
 export interface FormatterSettings {
   SplitAttributesOnFormat: boolean;

diff --git a/packages/settings/src/settings.ts b/packages/settings/src/settings.ts
@@ -9,7 +9,7 @@ const defaultSettings: Settings = {
   logging: { level: "error" },
   SAPUI5WebServer: "",
   SplitAttributesOnFormat: true,
-  ReportNonUniqueIdsCrossViewFiles: true,
+  LimitUniqueIdDiagnostics: false,
 };
 deepFreezeStrict(defaultSettings);
 

diff --git a/packages/settings/test/unit/settings.test.ts b/packages/settings/test/unit/settings.test.ts
@@ -46,7 +46,7 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setGlobalSettings(globalSettings);
       const docSettings = await getSettingsForDocument("doc1");
@@ -59,7 +59,7 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setGlobalSettings(globalSettings);
       const docSettings = await getSettingsForDocument("doc1");
@@ -77,7 +77,7 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setSettingsForDocument("doc1", Promise.resolve(docSettings));
       const result = await getSettingsForDocument("doc1");
@@ -94,7 +94,7 @@ describe("settings utilities", () => {
           trace: { server: "off" as const },
           logging: { level: "off" as const },
           SplitAttributesOnFormat: true,
-          ReportNonUniqueIdsCrossViewFiles: true,
+          LimitUniqueIdDiagnostics: false,
         })
       );
       expect(hasSettingsForDocument("doc1")).toBeTrue();
@@ -111,7 +111,7 @@ describe("settings utilities", () => {
           trace: { server: "off" as const },
           logging: { level: "off" as const },
           SplitAttributesOnFormat: true,
-          ReportNonUniqueIdsCrossViewFiles: true,
+          LimitUniqueIdDiagnostics: false,
         })
       );
       expect(hasSettingsForDocument("doc1")).toBeFalse();
@@ -125,7 +125,7 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setSettingsForDocument("doc1", Promise.resolve(docSettings));
       expect(await getSettingsForDocument("doc1")).toStrictEqual(docSettings);
@@ -137,14 +137,14 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       const docSettings2 = {
         codeAssist: { deprecated: true, experimental: false },
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setSettingsForDocument("doc1", Promise.resolve(docSettings1));
       setSettingsForDocument("doc1", Promise.resolve(docSettings2));
@@ -164,14 +164,14 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       const docSettings2 = {
         codeAssist: { deprecated: true, experimental: false },
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setSettingsForDocument("doc1", Promise.resolve(docSettings1));
       setSettingsForDocument("doc2", Promise.resolve(docSettings2));
@@ -197,7 +197,7 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setSettingsForDocument("doc1", Promise.resolve(docSettings));
 
@@ -213,14 +213,14 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       const docSettings2 = {
         codeAssist: { deprecated: true, experimental: false },
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setSettingsForDocument("doc1", Promise.resolve(docSettings1));
       setSettingsForDocument("doc2", Promise.resolve(docSettings2));
@@ -237,7 +237,7 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setGlobalSettings(globalSettings);
       expect(await getSettingsForDocument("doc1")).toStrictEqual(
@@ -253,7 +253,7 @@ describe("settings utilities", () => {
         trace: { server: "off" as const },
         logging: { level: "off" as const },
         SplitAttributesOnFormat: true,
-        ReportNonUniqueIdsCrossViewFiles: true,
+        LimitUniqueIdDiagnostics: false,
       };
       setConfigurationSettings(configSettings);
       expect(getConfigurationSettings()).toStrictEqual(configSettings);

diff --git a/packages/vscode-ui5-language-assistant/README.md b/packages/vscode-ui5-language-assistant/README.md
@@ -97,7 +97,7 @@ and cannot be configured by the end user.
 
 #### Relevant User/Workspace settings for Duplicate ID tags
 
-- `UI5LanguageAssistant.ReportNonUniqueIdsCrossViewFiles` is set on by default and report diagnostics cross view files. Settings it to false, reports only on a single file
+- `UI5LanguageAssistant.LimitUniqueIdDiagnostics` Set this setting to `true` to limit diagnostics reporting for unique IDs on a single file.
 
 ### XML View Quick Fix
 

diff --git a/packages/vscode-ui5-language-assistant/package.json b/packages/vscode-ui5-language-assistant/package.json
@@ -90,11 +90,11 @@
           "default": true,
           "markdownDescription": "Put each attribute on a new line when formatting `*.view.xml` or `*.fragment.xml`."
         },
-        "UI5LanguageAssistant.ReportNonUniqueIdsCrossViewFiles": {
+        "UI5LanguageAssistant.LimitUniqueIdDiagnostics": {
           "type": "boolean",
           "scope": "window",
-          "default": true,
-          "markdownDescription": "Diagnostic report for non unique ID on `*.view.xml` or `*.fragment.xml`."
+          "default": false,
+          "markdownDescription": "Limit diagnostics for unique IDs to current `*.view.xml` or `*.fragment.xml` file"
         },
         "UI5LanguageAssistant.SAPUI5WebServer": {
           "type": "string",

diff --git a/packages/xml-views-validation/src/validators/non-unique-id.ts b/packages/xml-views-validation/src/validators/non-unique-id.ts
@@ -25,11 +25,11 @@ const isNoneUniqueIdIssue = (
   currentDocIssues: ControlIdLocation[]
 ): boolean => {
   const settings = getConfigurationSettings();
-  const reportNonUniqueIds = settings.ReportNonUniqueIdsCrossViewFiles;
-  if (reportNonUniqueIds) {
-    return ctrId.length > 1;
+  const limitDiagReport = settings.LimitUniqueIdDiagnostics;
+  if (limitDiagReport) {
+    return ctrId.length > 1 && currentDocIssues.length > 1;
   }
-  return ctrId.length > 1 && currentDocIssues.length > 1;
+  return ctrId.length > 1;
 };
 
 const getIdenticalIDsRanges = (
@@ -39,14 +39,14 @@ const getIdenticalIDsRanges = (
   uri: string;
 }[] => {
   const settings = getConfigurationSettings();
-  const reportNonUniqueIds = settings.ReportNonUniqueIdsCrossViewFiles;
-  if (reportNonUniqueIds) {
-    return map(otherDocsIssues, (_) => ({
-      range: _.range,
-      uri: _.uri,
-    }));
+  const limitDiagReport = settings.LimitUniqueIdDiagnostics;
+  if (limitDiagReport) {
+    return [];
   }
-  return [];
+  return map(otherDocsIssues, (_) => ({
+    range: _.range,
+    uri: _.uri,
+  }));
 };
 /**
  * Validates non-unique control IDs within the specified context

diff --git a/packages/xml-views-validation/test/unit/validators/non-unique-id.test.ts b/packages/xml-views-validation/test/unit/validators/non-unique-id.test.ts
@@ -325,11 +325,11 @@ describe("the use of non unique id validation", () => {
         },
       ]);
     });
-    it("will not detect duplicate IDs cross view files when UI5LanguageAssistant.ReportNonUniqueIdsCrossViewFiles settings set to false", async () => {
+    it("will not detect duplicate IDs cross view files when UI5LanguageAssistant.LimitUniqueIdDiagnostics settings set to false", async () => {
       // arrange
       setConfigurationSettings({
         ...getDefaultSettings(),
-        ReportNonUniqueIdsCrossViewFiles: false,
+        LimitUniqueIdDiagnostics: true,
       });
       const xmlSnippet = `
         <mvc:View