Skip to content

Commit

Permalink
Incorporate PR review
Browse files Browse the repository at this point in the history
- Use 'id' consistently instead of 'name' even for descriptions
- Ignore generated files with prettier
- Ensure we give warning if ID field is missing
  • Loading branch information
martin-fleck-at committed Nov 30, 2023
1 parent 36de6fc commit 8c38379
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 17 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# keep in sync with .prettierignore if useful
node_modules
dist
out
Expand Down
7 changes: 7 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# keep in sync with .eslintignore if useful
node_modules
dist
out
lib
examples
generated
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ export interface IdProvider extends NameProvider {
}

/**
* A name provider that returns the fully qualified name of a node by default but also exposes methods to get other names:
* - The local name is just the name of the node itself if it has a name.
* - The qualified name / document-local name is the name of the node itself plus all it's named parents within the document
* - The fully qualified is the package name plus the document-local name.
* A name provider that returns the fully qualified ID of a node by default but also exposes methods to get other names:
* - The Node ID is just the id of the node itself if it has an id.
* - The Local ID is the Node ID itself plus the Node ID of all it's parents within the same document.
* - The External ID is the Local ID prefixed with the package name.
*/
export class DefaultIdProvider implements NameProvider, IdProvider {
constructor(
Expand Down
20 changes: 10 additions & 10 deletions extensions/crossmodel-lang/src/language-server/cross-model-scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,29 +83,29 @@ export class CrossModelScopeComputation extends DefaultScopeComputation {
const packageInfo = this.packageManager.getPackageInfoByDocument(document);
const packageId = packageInfo?.id ?? UNKNOWN_PROJECT_ID;
const packageName = packageInfo?.referenceName ?? UNKNOWN_PROJECT_REFERENCE;
const packageQualifiedName = this.idProvider.getExternalId(node, packageName);
const externalId = this.idProvider.getExternalId(node, packageName);

// Export nodes twice: Once for external usage with the fully-qualified name and once for package-local usage.
// To avoid duplicates in the UI but still allow access to the node through both names we filter the
// external usage descriptions in the CrossModelCompletionProvider if package-local usage is also available

let description: AstNodeDescription | undefined;
if (packageQualifiedName) {
description = this.descriptions.createDescription(node, packageQualifiedName, document);
exports.push(new PackageExternalAstNodeDescription(packageId, packageQualifiedName, description));
if (externalId) {
description = this.descriptions.createDescription(node, externalId, document);
exports.push(new PackageExternalAstNodeDescription(packageId, externalId, description));
}
const packageLocalName = this.idProvider.getLocalId(node);
if (packageLocalName && description) {
exports.push(new PackageLocalAstNodeDescription(packageId, packageLocalName, description));
const localId = this.idProvider.getLocalId(node);
if (localId && description) {
exports.push(new PackageLocalAstNodeDescription(packageId, localId, description));
}
}

protected override processNode(node: AstNode, document: LangiumDocument, scopes: PrecomputedScopes): void {
const container = node.$container;
if (container) {
const name = this.idProvider.getNodeId(node);
if (name) {
scopes.add(container, this.descriptions.createDescription(node, name, document));
const id = this.idProvider.getNodeId(node);
if (id) {
scopes.add(container, this.descriptions.createDescription(node, id, document));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,16 @@
import { AstNode, ValidationAcceptor, ValidationChecks } from 'langium';
import type { CrossModelServices } from './cross-model-module.js';
import { ID_PROPERTY } from './cross-model-naming.js';
import { CrossModelAstType, DiagramEdge } from './generated/ast.js';
import {
CrossModelAstType,
DiagramEdge,
isDiagramEdge,
isDiagramNode,
isEntity,
isEntityAttribute,
isRelationship,
isSystemDiagram
} from './generated/ast.js';

/**
* Register custom validation checks.
Expand All @@ -29,15 +38,29 @@ export class CrossModelValidator {
checkUniqueId(node: AstNode, accept: ValidationAcceptor): void {
const elementName = this.services.references.IdProvider.getNodeId(node);
if (!elementName) {
if (this.shouldHaveId(node)) {
accept('error', 'Missing required id field', { node, property: ID_PROPERTY });
}
return;
}
const allElements = this.services.shared.workspace.IndexManager.allElements();
const duplicates = allElements.filter(description => description.name === elementName).toArray();
const allElements = Array.from(this.services.shared.workspace.IndexManager.allElements());
const duplicates = allElements.filter(description => description.name === elementName);
if (duplicates.length > 1) {
accept('error', 'Must provide a unique id.', { node, property: ID_PROPERTY });
}
}

protected shouldHaveId(node: AstNode): boolean {
return (
isEntity(node) ||
isEntityAttribute(node) ||
isRelationship(node) ||
isSystemDiagram(node) ||
isDiagramEdge(node) ||
isDiagramNode(node)
);
}

checkDiagramEdge(edge: DiagramEdge, accept: ValidationAcceptor): void {
if (edge.sourceNode?.ref?.entity?.ref?.$type !== edge.relationship?.ref?.parent?.ref?.$type) {
accept('error', 'Source must match type of parent', { node: edge, property: 'sourceNode' });
Expand Down

0 comments on commit 8c38379

Please sign in to comment.