feat: user settings to control diagnostics reporting cross view files (…

…#734) * feat: user settings to control diagnostics reporting cross different view files * fix: add change set * fix: review comment * fix: update readme file
SAP · Oct 16, 2024 · 3abf9a6 · 3abf9a6
1 parent 3fa9e9b
commit 3abf9a6
Show file tree

Hide file tree

Showing 17 changed files with 225 additions and 13 deletions.
diff --git a/.changeset/ten-rivers-pull.md b/.changeset/ten-rivers-pull.md
@@ -0,0 +1,12 @@
+---
+"@ui5-language-assistant/vscode-ui5-language-assistant-bas-ext": patch
+"vscode-ui5-language-assistant": patch
+"@ui5-language-assistant/xml-views-completion": patch
+"@ui5-language-assistant/language-server": patch
+"@ui5-language-assistant/settings": patch
+"@ui5-language-assistant/binding": patch
+"@ui5-language-assistant/context": patch
+"@ui5-language-assistant/fe": patch
+---
+
+feat: User settings to control diagnostics reporting cross view files
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,6 +39,7 @@ describe("aggregation binding", () => {
  server: "off",
  },
  SplitAttributesOnFormat: 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,6 +44,7 @@ describe("index", () => {
  server: "off",
  },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
 
  beforeAll(async function () {

diff --git a/packages/context/api.d.ts b/packages/context/api.d.ts
@@ -5,5 +5,6 @@ export type {
  ManifestDetails,
  YamlDetails,
  ProjectKind,
+ ControlIdLocation,
 } from "./src/types";
 export type { Manifest } from "@sap-ux/project-access";
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,6 +45,7 @@ describe("contextPath attribute value completion", () => {
  server: "off",
  },
  SplitAttributesOnFormat: 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,6 +43,7 @@ describe("filterBar id attribute value completion", () => {
  server: "off",
  },
  SplitAttributesOnFormat: 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,6 +48,7 @@ describe("metaPath attribute value completion", () => {
  server: "off",
  },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
 
  const annotationSnippetCDS = `

diff --git a/packages/language-server/src/server.ts b/packages/language-server/src/server.ts
@@ -24,6 +24,7 @@ import {
  getSettingsForDocument,
  setConfigurationSettings,
  Settings,
+ getConfigurationSettings,
 } from "@ui5-language-assistant/settings";
 import { commands } from "@ui5-language-assistant/user-facing-text";
 import { ServerInitializationOptions } from "../api";
@@ -44,6 +45,7 @@ import {
  reactOnViewFileChange,
  reactOnPackageJson,
  isContext,
+ Context,
 } from "@ui5-language-assistant/context";
 import { diagnosticToCodeActionFix } from "./quick-fix";
 import { executeCommand } from "./commands";
@@ -296,6 +298,28 @@ const validateIdsOfOpenDocuments = async (): Promise<void> => {
  connection.sendDiagnostics({ uri: document.uri, diagnostics });
  }
 };
+/**
+ * Validates the IDs of the open document and sends the diagnostics to the client
+ * @param {TextDocument} document - The open document to be validated
+ * @param {Context} context - The context containing additional information
+ * @returns {void}
+ */
+const validateIdsOfOpenDocument = (
+ document: TextDocument,
+ context: Context
+): void => {
+ const idDiagnostics = getXMLViewIdDiagnostics({
+ document,
+ context,
+ });
+ let diagnostics = documentsDiagnostics.get(document.uri) ?? [];
+ diagnostics = diagnostics.concat(idDiagnostics);
+
+ getLogger().trace("computed diagnostics", {
+ diagnostics,
+ });
+ connection.sendDiagnostics({ uri: document.uri, diagnostics });
+};
 
 async function validateOpenDocumentsOnDidChangeWatchedFiles(
  changes: FileEvent[]
@@ -391,7 +415,13 @@ documents.onDidChangeContent(async (changeEvent): Promise<void> => {
  context,
  });
  documentsDiagnostics.set(document.uri, diagnostics);
- await validateIdsOfOpenDocuments();
+ const settings = getConfigurationSettings();
+ const limitUniqueIdsDiagReport = settings.LimitUniqueIdDiagnostics;
+ if (limitUniqueIdsDiagReport) {
+ validateIdsOfOpenDocument(document, context);
+ } else {
+ await validateIdsOfOpenDocuments();
+ }
  }
 });
 
@@ -466,7 +496,7 @@ function ensureDocumentSettingsUpdated(resource: string): void {
  }
 }
 
-connection.onDidChangeConfiguration((change) => {
+connection.onDidChangeConfiguration(async (change) => {
  getLogger().debug("`onDidChangeConfiguration` event");
  if (hasConfigurationCapability) {
  getLogger().trace("Reset all cached document settings");
@@ -487,9 +517,8 @@ connection.onDidChangeConfiguration((change) => {
  });
  setConfigurationSettings(ui5LangAssistSettings);
  }
- // In the future we might want to
  // re-validate the files related to the `cached document settings`.
-
+ await validateIdsOfOpenDocuments();
  // `setLogLevel` will ignore `undefined` values
  setLogLevel(change?.settings?.UI5LanguageAssistant?.logging?.level);
 });

diff --git a/packages/settings/api.d.ts b/packages/settings/api.d.ts
@@ -13,11 +13,15 @@ export type Settings = CodeAssistSettings &
  TraceSettings &
  LoggingSettings &
  WebServerSettings &
+ LimitUniqueIdDiagnostics &
  FormatterSettings;
 
 export interface WebServerSettings {
  SAPUI5WebServer?: string;
 }
+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,6 +9,7 @@ const defaultSettings: Settings = {
  logging: { level: "error" },
  SAPUI5WebServer: "",
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
 };
 deepFreezeStrict(defaultSettings);
 

diff --git a/packages/settings/test/unit/settings.test.ts b/packages/settings/test/unit/settings.test.ts
@@ -46,6 +46,7 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  setGlobalSettings(globalSettings);
  const docSettings = await getSettingsForDocument("doc1");
@@ -58,6 +59,7 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  setGlobalSettings(globalSettings);
  const docSettings = await getSettingsForDocument("doc1");
@@ -75,6 +77,7 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  setSettingsForDocument("doc1", Promise.resolve(docSettings));
  const result = await getSettingsForDocument("doc1");
@@ -91,6 +94,7 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  })
  );
  expect(hasSettingsForDocument("doc1")).toBeTrue();
@@ -107,6 +111,7 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  })
  );
  expect(hasSettingsForDocument("doc1")).toBeFalse();
@@ -120,6 +125,7 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  setSettingsForDocument("doc1", Promise.resolve(docSettings));
  expect(await getSettingsForDocument("doc1")).toStrictEqual(docSettings);
@@ -131,12 +137,14 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  const docSettings2 = {
  codeAssist: { deprecated: true, experimental: false },
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  setSettingsForDocument("doc1", Promise.resolve(docSettings1));
  setSettingsForDocument("doc1", Promise.resolve(docSettings2));
@@ -156,12 +164,14 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  const docSettings2 = {
  codeAssist: { deprecated: true, experimental: false },
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  setSettingsForDocument("doc1", Promise.resolve(docSettings1));
  setSettingsForDocument("doc2", Promise.resolve(docSettings2));
@@ -187,6 +197,7 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  setSettingsForDocument("doc1", Promise.resolve(docSettings));
 
@@ -202,12 +213,14 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  const docSettings2 = {
  codeAssist: { deprecated: true, experimental: false },
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  setSettingsForDocument("doc1", Promise.resolve(docSettings1));
  setSettingsForDocument("doc2", Promise.resolve(docSettings2));
@@ -224,6 +237,7 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: true,
+ LimitUniqueIdDiagnostics: false,
  };
  setGlobalSettings(globalSettings);
  expect(await getSettingsForDocument("doc1")).toStrictEqual(
@@ -239,6 +253,7 @@ describe("settings utilities", () => {
  trace: { server: "off" as const },
  logging: { level: "off" as const },
  SplitAttributesOnFormat: 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
@@ -95,6 +95,10 @@ and cannot be configured by the end user.
  - References to filter bars that cannot be found in current file
  - Use of context paths that do not lead to the annotations of types expected for the given building block
 
+#### Relevant User/Workspace
+
+- `UI5LanguageAssistant.LimitUniqueIdDiagnostics` is set off by default to check all `*.fragment.xml` and `*.view.xml` files for unique IDs. If you like to limit this validation to the current file, set it on.
+
 ### XML View Quick Fix
 
 #### Description

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

diff --git a/packages/vscode-ui5-language-assistant/test/manual-tests/unique-control-ids.md b/packages/vscode-ui5-language-assistant/test/manual-tests/unique-control-ids.md
@@ -2,7 +2,7 @@
 
 ## Associated user stories:
 
-[#646](https://github.com/SAP/ui5-language-assistant/issues/646) Generate ID's: make this unique for a whole project
+[#646](https://github.com/SAP/ui5-language-assistant/issues/646) Generate ID's: make this unique for a whole project [#729](https://github.com/SAP/ui5-language-assistant/issues/729) User settings to control diagnostics reporting cross different view files
 
 ## Install latest UI5 Language Assistant
 
@@ -61,7 +61,7 @@ You can navigate to related file where identical ids are used
 **Note:** For this step test, we use `test-packages/framework/projects/adp.test` project
 
 1. Open `actionToolbar.fragment.xml` file which is located under `adp.test/webapp/changes/fragments` folder
-2. Copy and replace snippet below with `<!--  add your xml here -->`
+2. Copy and replace snippet below with `<!-- add your xml here -->`
 
 ```xml
  <Text id="_IDGenText" >
@@ -77,3 +77,27 @@ You can navigate to related file where identical ids are used
 ##### Note
 
 You can navigate to related file where identical ids are used
+
+## User setting to control diagnostics reporting
+
+1. Open user settings by clicking `view>Command Pallette...` and type `Preferences: Open User settings`. Select first option. Search for `Report Non Unique Ids Cross View Files` and uncheck checkbox. (By default it reports diagnostics cross view files)
+2. Open `Main.view.xml` file which is located under `app/manage_travels/webapp/ext/main` folder
+3. Copy and paste snippet below inside `<Content>` tag
+
+```xml
+ <Text id="_IDGenText" >
+ <Button id="_IDGenButton" />
+```
+
+4. Check that no diagnostic is reported as there is not duplicate id
+5. Create a new file e.g `MyTest.view.xml` under webapp and copy paste content of `Main.view.xml` file in `MyTest.view.xml`
+6. Check that no diagnostics is reported for duplicate ids cross view files
+7. Add `<Text id="_IDGenText" >` to `Main.view.xml`
+8. Check that diagnostic is reported for duplicate ids of same file
+9. Reset checkbox of `Report Non Unique Ids Cross View Files`. See instruction from #1 above
+10. Check that diagnostics are reported for duplicate ids across view files
+
+## Quick fix with "Report Non Unique Ids Cross View Files" user settings
+
+1. Open user settings by clicking `view>Command Pallette...` and type `Preferences: Open User settings`. Select first option. Search for `Report Non Unique Ids Cross View Files` and uncheck checkbox. (By default it reports diagnostics cross view files)
+2. [Follow steps](#quick-fix). Despite user settings, behavior should not be affected
diff --git a/packages/xml-views-validation/package.json b/packages/xml-views-validation/package.json
@@ -21,6 +21,7 @@
  "license": "Apache-2.0",
  "typings": "./api.d.ts",
  "dependencies": {
+ "@ui5-language-assistant/settings": "4.0.9",
  "@ui5-language-assistant/constant": "0.0.1",
  "@ui5-language-assistant/context": "4.0.30",
  "@ui5-language-assistant/logic-utils": "4.0.20",