Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type Usage Consistency (2) #548

Merged
merged 8 commits into from
Jul 12, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/langium/src/grammar/langium-grammar-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import * as ast from './generated/ast';
import { isParserRule, isRuleCall } from './generated/ast';
import { findKeywordNode, findNameAssignment, getEntryRule, getTypeName, isDataTypeRule, isOptional, resolveImport, resolveTransitiveImports, terminalRegex } from './grammar-util';
import type { LangiumGrammarServices } from './langium-grammar-module';
import { collectInferredTypes } from './type-system/inferred-types';
import { applyErrorToAssignment, collectAllInterfaces, InterfaceInfo, validateTypesConsistency } from './type-system/type-validator';

export class LangiumGrammarValidationRegistry extends ValidationRegistry {
Expand Down Expand Up @@ -415,10 +416,22 @@ export class LangiumGrammarValidator {
accept('error', 'Rules are not allowed to return union types.', { node: rule, property: 'returnType' });
}
}

for (const interfaceType of grammar.interfaces) {
interfaceType.superTypes.forEach((superType, i) => {
if (superType.ref && ast.isType(superType.ref)) {
accept('error', 'Interfaces cannot extend union types.', { node: interfaceType, property: 'superTypes', index: i });
} else if(superType.ref && ast.isParserRule(superType.ref)) {
// collect just the beginning of whatever inferred types this standalone rule produces
// looking to exclude anything that would be a union down the line
const inferred = collectInferredTypes([superType.ref as ast.ParserRule], []);
if(inferred.unions.length > 0) {
// inferred union type also cannot be extended
accept('error', `An interface cannot extend a union type, which was inferred from parser rule ${superType.ref.name}.`, { node: interfaceType, property: 'superTypes', index: i });
} else {
// otherwise we'll allow it, but issue a warning against basing declared off of inferred types
accept('warning', 'Extending an interface by a parser rule gives an ambiguous type, instead of the expected declared type.', { node: interfaceType, property: 'superTypes', index: i });
}
}
});
}
Expand Down
33 changes: 32 additions & 1 deletion packages/langium/test/grammar/langium-grammar-validator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { createLangiumGrammarServices } from '../../src';
import { Assignment, Grammar, ParserRule } from '../../src/grammar/generated/ast';
import { expectError, validationHelper } from '../../src/test';
import { expectError, expectWarning, validationHelper } from '../../src/test';

const services = createLangiumGrammarServices();
const validate = validationHelper<Grammar>(services.grammar);
Expand All @@ -30,4 +30,35 @@ describe('Langium grammar validation', () => {
property: {name: 'terminal'}
});
});

test('Interfaces should only be able to extend inferred interfaces', async () => {
const validationResult = await validate(`
grammar G
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would vote for speaking names, it is hard to read


entry SA: S1 | S2;
SB: S1;

S1: 's1' s1=ID;
S2: 's2' s2=ID;

interface DA extends SA {}
interface D2 extends S2 {}
interface DB extends SB {}
msujew marked this conversation as resolved.
Show resolved Hide resolved

hidden terminal WS: /\\s+/;
terminal ID: /[a-zA-Z_][a-zA-Z0-9_]*/;
`);
expectError(validationResult, /An interface cannot extend a union type, which was inferred from parser rule SA./, {
node: validationResult.document.parseResult.value.interfaces[0],
property: {name: 'superTypes'}
});
expectWarning(validationResult, /Extending an interface by a parser rule gives an ambiguous type, instead of the expected declared type./, {
node: validationResult.document.parseResult.value.interfaces[1],
property: {name: 'superTypes'}
});
expectError(validationResult, /An interface cannot extend a union type, which was inferred from parser rule SB./, {
node: validationResult.document.parseResult.value.interfaces[2],
property: {name: 'superTypes'}
});
});
});