Validation: The filename and it's id in the file should be in sync (C…

…rossBreezeNL#76) - Add warning if filename and element id do not match - Provide quick fix to properly rename file Minor: - Fix watch scripts for browser and electron to use package name - Fix wrong translation of diagnostic severity
eclipsesource · Dec 6, 2024 · ff0669f · ff0669f
1 parent 023d330
commit ff0669f
Show file tree

Hide file tree

Showing 14 changed files with 180 additions and 23 deletions.
diff --git a/extensions/crossmodel-lang/src/glsp-server/common/cross-model-storage.ts b/extensions/crossmodel-lang/src/glsp-server/common/cross-model-storage.ts
@@ -23,6 +23,7 @@ import {
 import { inject, injectable, postConstruct } from 'inversify';
 import { AstUtils } from 'langium';
 import debounce from 'p-debounce';
+import { DiagnosticSeverity } from 'vscode-languageserver-protocol';
 import { URI } from 'vscode-uri';
 import { CrossModelRoot } from '../../language-server/generated/ast.js';
 import { AstCrossModelDocument } from '../../model-server/open-text-document-manager.js';
@@ -84,7 +85,10 @@ export class CrossModelStorage implements SourceModelStorage, ClientSessionListe
    protected async updateEditMode(document: AstCrossModelDocument): Promise<Action[]> {
       const actions = [];
       const prevEditMode = this.state.editMode;
-      this.state.editMode = document.diagnostics.length > 0 ? EditMode.READONLY : EditMode.EDITABLE;
+      this.state.editMode =
+         document.diagnostics.filter(diagnostic => diagnostic.severity === DiagnosticSeverity.Error).length > 0
+            ? EditMode.READONLY
+            : EditMode.EDITABLE;
       if (prevEditMode !== this.state.editMode) {
          if (this.state.isReadonly) {
             actions.push(SetEditModeAction.create(EditMode.READONLY));

diff --git a/extensions/crossmodel-lang/src/language-server/cross-model-code-action-provider.ts b/extensions/crossmodel-lang/src/language-server/cross-model-code-action-provider.ts
@@ -0,0 +1,44 @@
+/********************************************************************************
+ * Copyright (c) 2024 CrossBreeze.
+ ********************************************************************************/
+
+import { ModelFileExtensions } from '@crossbreeze/protocol';
+import { UriUtils, type LangiumDocument } from 'langium';
+import type { CodeActionProvider } from 'langium/lsp';
+import { RenameFile, type CodeAction, type CodeActionParams } from 'vscode-languageserver-protocol';
+import { FilenameNotMatchingDiagnostic } from './cross-model-validator.js';
+import { findSemanticRoot } from './util/ast-util.js';
+
+export class CrossModelCodeActionProvider implements CodeActionProvider {
+   getCodeActions(document: LangiumDocument, params: CodeActionParams): CodeAction[] {
+      const result: CodeAction[] = [];
+      const accept = (action: CodeAction | undefined): void => {
+         if (action) {
+            result.push(action);
+         }
+      };
+      for (const diagnostic of params.context.diagnostics) {
+         if (FilenameNotMatchingDiagnostic.is(diagnostic)) {
+            accept(this.fixFilenameNotMatching(diagnostic, document));
+         }
+      }
+      return result;
+   }
+
+   private fixFilenameNotMatching(diagnostic: FilenameNotMatchingDiagnostic, document: LangiumDocument): CodeAction | undefined {
+      const semanticRoot = findSemanticRoot(document);
+      if (!semanticRoot || !semanticRoot.id) {
+         return undefined;
+      }
+      const newName = semanticRoot.id + ModelFileExtensions.getFileExtension(document.uri.toString());
+      const newUri = UriUtils.joinPath(UriUtils.dirname(document.uri), newName);
+      return {
+         title: `Rename file to '${newName}'`,
+         edit: {
+            documentChanges: [RenameFile.create(document.uri.toString(), newUri.toString())]
+         },
+         kind: 'quickfix',
+         diagnostics: [diagnostic]
+      };
+   }
+}
diff --git a/extensions/crossmodel-lang/src/language-server/cross-model-module.ts b/extensions/crossmodel-lang/src/language-server/cross-model-module.ts
@@ -20,6 +20,7 @@ import { OpenTextDocumentManager } from '../model-server/open-text-document-mana
 import { OpenableTextDocuments } from '../model-server/openable-text-documents.js';
 import { Serializer } from '../model-server/serializer.js';
 import { ClientLogger } from './cross-model-client-logger.js';
+import { CrossModelCodeActionProvider } from './cross-model-code-action-provider.js';
 import { CrossModelCompletionProvider } from './cross-model-completion-provider.js';
 import { CrossModelDocumentBuilder } from './cross-model-document-builder.js';
 import { CrossModelModelFormatter } from './cross-model-formatter.js';
@@ -149,6 +150,9 @@ export interface CrossModelAddedServices {
    parser: {
       TokenBuilder: CrossModelTokenBuilder;
    };
+   lsp: {
+      /* implement */ CodeActionProvider: CrossModelCodeActionProvider;
+   };
    /* override */ shared: CrossModelSharedServices;
 }
 
@@ -179,6 +183,7 @@ export function createCrossModelModule(
          CrossModelValidator: services => new CrossModelValidator(services)
       },
       lsp: {
+         CodeActionProvider: () => new CrossModelCodeActionProvider(),
          CompletionProvider: services => new CrossModelCompletionProvider(services),
          Formatter: () => new CrossModelModelFormatter()
       },

diff --git a/extensions/crossmodel-lang/src/language-server/cross-model-validator.ts b/extensions/crossmodel-lang/src/language-server/cross-model-validator.ts
@@ -1,14 +1,20 @@
 /********************************************************************************
  * Copyright (c) 2023 CrossBreeze.
  ********************************************************************************/
-import { findAllExpressions, getExpression, getExpressionPosition, getExpressionText } from '@crossbreeze/protocol';
-import { AstNode, GrammarUtils, Reference, ValidationAcceptor, ValidationChecks } from 'langium';
+import { findAllExpressions, getExpression, getExpressionPosition, getExpressionText, ModelFileExtensions } from '@crossbreeze/protocol';
+import { AstNode, GrammarUtils, Reference, UriUtils, ValidationAcceptor, ValidationChecks } from 'langium';
+import { Diagnostic } from 'vscode-languageserver-protocol';
 import type { CrossModelServices } from './cross-model-module.js';
 import { ID_PROPERTY, IdentifiableAstNode } from './cross-model-naming.js';
 import {
    Attribute,
    AttributeMapping,
    CrossModelAstType,
+   isEntity,
+   isEntityAttribute,
+   isMapping,
+   isRelationship,
+   isSystemDiagram,
    Mapping,
    Relationship,
    RelationshipEdge,
@@ -17,14 +23,25 @@ import {
    SourceObjectCondition,
    SourceObjectDependency,
    TargetObject,
-   TargetObjectAttribute,
-   isEntity,
-   isEntityAttribute,
-   isMapping,
-   isRelationship,
-   isSystemDiagram
+   TargetObjectAttribute
 } from './generated/ast.js';
-import { findDocument, getOwner } from './util/ast-util.js';
+import { findDocument, getOwner, isSemanticRoot } from './util/ast-util.js';
+
+export namespace CrossModelIssueCodes {
+   export const FilenameNotMatching = 'filename-not-matching';
+}
+
+export interface FilenameNotMatchingDiagnostic extends Diagnostic {
+   data: {
+      code: typeof CrossModelIssueCodes.FilenameNotMatching;
+   };
+}
+
+export namespace FilenameNotMatchingDiagnostic {
+   export function is(diagnostic: Diagnostic): diagnostic is FilenameNotMatchingDiagnostic {
+      return diagnostic.data?.code === CrossModelIssueCodes.FilenameNotMatching;
+   }
+}
 
 /**
  * Register custom validation checks.
@@ -56,6 +73,31 @@ export class CrossModelValidator {
    checkNode(node: AstNode, accept: ValidationAcceptor): void {
       this.checkUniqueGlobalId(node, accept);
       this.checkUniqueNodeId(node, accept);
+      this.checkMatchingFilename(node, accept);
+   }
+
+   protected checkMatchingFilename(node: AstNode, accept: ValidationAcceptor): void {
+      if (!isSemanticRoot(node)) {
+         return;
+      }
+      if (!node.id) {
+         // diagrams may not have ids set and therefore are not required to match the filename
+         return;
+      }
+      const document = findDocument(node);
+      if (!document) {
+         return;
+      }
+      const basename = UriUtils.basename(document.uri);
+      const extname = ModelFileExtensions.getFileExtension(basename) ?? UriUtils.extname(document.uri);
+      const basenameWithoutExt = basename.slice(0, -extname.length);
+      if (basenameWithoutExt.toLowerCase() !== node.id.toLocaleLowerCase()) {
+         accept('warning', `Filename should match element id: ${node.id}`, {
+            node,
+            property: ID_PROPERTY,
+            data: { code: CrossModelIssueCodes.FilenameNotMatching }
+         });
+      }
    }
 
    protected checkUniqueGlobalId(node: AstNode, accept: ValidationAcceptor): void {

diff --git a/extensions/crossmodel-lang/src/language-server/util/ast-util.ts b/extensions/crossmodel-lang/src/language-server/util/ast-util.ts
@@ -172,6 +172,10 @@ export type WithDocument<T> = T & { $document: LangiumDocument<CrossModelRoot> }
 export type DocumentContent = LangiumDocument | AstNode;
 export type TypeGuard<T> = (item: unknown) => item is T;
 
+export function isSemanticRoot(element: unknown): element is SemanticRoot {
+   return isEntity(element) || isMapping(element) || isRelationship(element) || isSystemDiagram(element);
+}
+
 export function findSemanticRoot(input: DocumentContent): SemanticRoot | undefined;
 export function findSemanticRoot<T extends SemanticRoot>(input: DocumentContent, guard: TypeGuard<T>): T | undefined;
 export function findSemanticRoot<T extends SemanticRoot>(input: DocumentContent, guard?: TypeGuard<T>): SemanticRoot | T | undefined {

diff --git a/extensions/crossmodel-lang/src/model-server/model-server.ts b/extensions/crossmodel-lang/src/model-server/model-server.ts
@@ -161,9 +161,9 @@ export class ModelServer implements Disposable {
          message: diagnostic.message,
          severity:
             diagnostic.severity === DiagnosticSeverity.Error
-               ? 'warning'
+               ? 'error'
                : diagnostic.severity === DiagnosticSeverity.Warning
-                 ? 'error'
+                 ? 'warning'
                  : 'info',
          type: langiumCode === 'lexing-error' ? 'lexing-error' : langiumCode === 'parsing-error' ? 'parsing-error' : 'validation-error'
       };

diff --git a/extensions/crossmodel-lang/test/language-server/cross-model-filename.test.ts b/extensions/crossmodel-lang/test/language-server/cross-model-filename.test.ts
@@ -0,0 +1,23 @@
+/********************************************************************************
+ * Copyright (c) 2024 CrossBreeze.
+ ********************************************************************************/
+
+import { ModelFileExtensions } from '@crossbreeze/protocol';
+import { entity1 } from './test-utils/test-documents/entity/entity1.js';
+import { createCrossModelTestServices, parseEntity, testUri } from './test-utils/utils.js';
+
+const services = createCrossModelTestServices();
+
+describe('CrossModel Filename Validation', () => {
+   test('Mismatching id and filename does not yield error', async () => {
+      const entity = await parseEntity({
+         services,
+         text: entity1,
+         validation: true,
+         documentUri: testUri('Customer2' + ModelFileExtensions.Entity)
+      });
+      expect(entity.id).toBe('Customer');
+      expect(entity.$document.diagnostics).toHaveLength(1);
+      expect(entity.$document.diagnostics![0].message).toContain('Filename should match element id');
+   });
+});
diff --git a/extensions/crossmodel-lang/test/language-server/cross-model-lang-relationship.test.ts b/extensions/crossmodel-lang/test/language-server/cross-model-lang-relationship.test.ts
@@ -12,8 +12,9 @@ import {
    relationship_with_attribute_wrong_entity,
    relationship_with_duplicate_attributes
 } from './test-utils/test-documents/relationship/index.js';
-import { createCrossModelTestServices, parseDocuments, parseRelationship } from './test-utils/utils.js';
+import { createCrossModelTestServices, parseDocuments, parseRelationship, testUri } from './test-utils/utils.js';
 
+import { ModelFileExtensions } from '@crossbreeze/protocol';
 import { address } from './test-utils/test-documents/entity/address.js';
 import { customer } from './test-utils/test-documents/entity/customer.js';
 import { order } from './test-utils/test-documents/entity/order.js';
@@ -48,19 +49,34 @@ describe('CrossModel language Relationship', () => {
    });
 
    test('relationship with attributes', async () => {
-      const relationship = await parseRelationship({ services, text: relationship_with_attribute, validation: true });
+      const relationship = await parseRelationship({
+         services,
+         text: relationship_with_attribute,
+         validation: true,
+         documentUri: testUri('Order_CustomerWithAttribute' + ModelFileExtensions.Relationship)
+      });
 
       expect(relationship.attributes).toHaveLength(1);
       expect(relationship.$document.diagnostics).toHaveLength(0);
    });
 
    test('relationship with wrong entity', async () => {
-      const relationship = await parseRelationship({ services, text: relationship_with_attribute_wrong_entity, validation: true });
+      const relationship = await parseRelationship({
+         services,
+         text: relationship_with_attribute_wrong_entity,
+         validation: true,
+         documentUri: testUri('Order_CustomerWithAttributeWrongEntity' + ModelFileExtensions.Relationship)
+      });
       expect(relationship.$document.diagnostics).toHaveLength(1);
    });
 
    test('relationship with duplicates', async () => {
-      const relationship = await parseRelationship({ services, text: relationship_with_duplicate_attributes, validation: true });
+      const relationship = await parseRelationship({
+         services,
+         text: relationship_with_duplicate_attributes,
+         validation: true,
+         documentUri: testUri('Order_CustomerWithDuplicateAttributes' + ModelFileExtensions.Relationship)
+      });
       expect(relationship.$document.diagnostics).toHaveLength(2);
    });
 });
diff --git a/package.json b/package.json
@@ -31,8 +31,8 @@
     "theia:bundle": "yarn theia:electron bundle && yarn theia:browser bundle",
     "theia:electron": "yarn --cwd applications/electron-app",
     "ui-test": "yarn --cwd e2e-tests ui-test",
-    "watch:browser": "lerna run --parallel --ignore=\"applications/electron-app\" watch",
-    "watch:electron": "lerna run --parallel --ignore=\"applications/browser-app\" watch"
+    "watch:browser": "lerna run --parallel --ignore=\"crossmodel-app\" watch",
+    "watch:electron": "lerna run --parallel --ignore=\"crossmodel-browser-app\" watch"
   },
   "devDependencies": {
     "@testing-library/react": "^11.2.7",

diff --git a/packages/core/src/browser/model-widget.tsx b/packages/core/src/browser/model-widget.tsx
@@ -3,7 +3,7 @@
  ********************************************************************************/
 
 import { ModelService, ModelServiceClient } from '@crossbreeze/model-service/lib/common';
-import { CrossModelDocument, CrossModelRoot, ModelUpdatedEvent, RenderProps } from '@crossbreeze/protocol';
+import { CrossModelDocument, CrossModelRoot, ModelDiagnostic, ModelUpdatedEvent, RenderProps } from '@crossbreeze/protocol';
 import {
    EntityComponent,
    ErrorView,
@@ -146,7 +146,7 @@ export class CrossModelWidget extends ReactWidget implements Saveable {
       if (doc === undefined) {
          throw new Error('Cannot save undefined model');
       }
-      if (doc.diagnostics.length > 0) {
+      if (ModelDiagnostic.hasErrors(doc.diagnostics)) {
          // we do not support saving erroneous models in model widgets as we cannot deal with them properly, fixes are done via code editor
          console.debug(`[${this.options.clientId}] Abort Save as we have an erroneous model`);
          return;

diff --git a/packages/protocol/src/model-service/protocol.ts b/packages/protocol/src/model-service/protocol.ts
@@ -211,6 +211,20 @@ export interface ModelDiagnostic {
    severity: 'error' | 'warning' | 'info';
 }
 
+export namespace ModelDiagnostic {
+   export function isError(diagnostic: ModelDiagnostic): boolean {
+      return diagnostic.severity === 'error';
+   }
+
+   export function errors(diagnostics: ModelDiagnostic[]): ModelDiagnostic[] {
+      return diagnostics.filter(isError);
+   }
+
+   export function hasErrors(diagnostics: ModelDiagnostic[]): boolean {
+      return diagnostics.some(isError);
+   }
+}
+
 export interface ModelUpdatedEvent<D = CrossModelDocument> {
    document: D;
    sourceClientId: string;

diff --git a/packages/protocol/src/model.ts b/packages/protocol/src/model.ts
@@ -113,6 +113,11 @@ export const ModelFileExtensions = {
       return undefined;
    },
 
+   getFileExtension(uri: string): string | undefined {
+      const fileType = this.getFileType(uri);
+      return !fileType ? undefined : ModelFileType.getFileExtension(fileType);
+   },
+
    getIconClass(uri: string): string | undefined {
       const fileType = this.getFileType(uri);
       if (!fileType) {

diff --git a/packages/react-model-ui/src/ModelContext.tsx b/packages/react-model-ui/src/ModelContext.tsx
@@ -69,7 +69,7 @@ export function useDirty(): boolean {
 }
 
 export function useReadonly(): boolean {
-   return useDiagnostics().length > 0;
+   return ModelDiagnostic.hasErrors(useDiagnostics());
 }
 
 export function useEntity(): Entity {

diff --git a/packages/react-model-ui/src/views/form/Header.tsx b/packages/react-model-ui/src/views/form/Header.tsx
@@ -2,7 +2,7 @@
  * Copyright (c) 2024 CrossBreeze.
  ********************************************************************************/
 
-import { ERRONEOUS_MODEL } from '@crossbreeze/protocol';
+import { ERRONEOUS_MODEL, ModelDiagnostic } from '@crossbreeze/protocol';
 import { OpenInNewOutlined, SaveOutlined } from '@mui/icons-material';
 import { AppBar, Box, Button, Icon, Toolbar, Typography } from '@mui/material';
 import { useDiagnostics, useDirty, useModelOpen, useModelSave } from '../../ModelContext';
@@ -23,7 +23,7 @@ export function Header({ name, id, iconClass }: HeaderProps): React.ReactElement
 
    return (
       <AppBar position='sticky'>
-         {diagnostics.length > 0 && createEditorError(ERRONEOUS_MODEL)}
+         {ModelDiagnostic.hasErrors(diagnostics) && createEditorError(ERRONEOUS_MODEL)}
          <Toolbar variant='dense' sx={{ minHeight: '40px' }}>
             <Box sx={{ display: { xs: 'none', sm: 'flex' }, flexGrow: 1, gap: '1em', alignItems: 'center' }}>
                {iconClass && <Icon baseClassName='codicon' className={iconClass} sx={{ fontSize: '1.7em !important' }} />}