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

- 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
CrossBreezeNL · Nov 29, 2024 · 253048d · 253048d
1 parent 6efa132
commit 253048d
Show file tree

Hide file tree

Showing 9 changed files with 146 additions and 17 deletions.
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,42 @@
+/********************************************************************************
+ * 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())]
+         }
+      };
+   }
+}
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,15 +23,26 @@ import {
    SourceObjectCondition,
    SourceObjectDependency,
    TargetObject,
-   TargetObjectAttribute,
-   isEntity,
-   isEntityAttribute,
-   isMapping,
-   isRelationship,
-   isSystemDiagram
+   TargetObjectAttribute
 } from './generated/ast.js';
 import { findDocument, getOwner } 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,27 @@ 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 (!isEntity(node) && !isMapping(node) && !isRelationship(node)) {
+         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/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/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/views/form/Header.tsx b/packages/react-model-ui/src/views/form/Header.tsx
@@ -23,7 +23,7 @@ export function Header({ name, id, iconClass }: HeaderProps): React.ReactElement
 
    return (
       <AppBar position='sticky'>
-         {diagnostics.length > 0 && createEditorError(ERRONEOUS_MODEL)}
+         {diagnostics.filter(diagnostic => diagnostic.severity === 'error').length > 0 && 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' }} />}