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

feat: various checks related to inheritance #633

Merged
merged 6 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 6 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,12 @@
"scopeName": "source.safe-ds",
"path": "./syntaxes/safe-ds.tmLanguage.json"
}
]
],
"configurationDefaults": {
"[safe-ds]": {
"editor.wordSeparators": "`~!@#$%^&*()-=+[]{}\\|;:'\",.<>/?»«"
}
}
},
"type": "module",
"main": "out/extension/main.cjs",
Expand Down
5 changes: 5 additions & 0 deletions src/language/helpers/nodeProperties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
SdsResult,
SdsResultList,
SdsStatement,
SdsType,
SdsTypeArgument,
SdsTypeArgumentList,
SdsTypeParameter,
Expand Down Expand Up @@ -165,6 +166,10 @@ export const parametersOrEmpty = (node: SdsCallable | undefined): SdsParameter[]
return node?.parameterList?.parameters ?? [];
};

export const parentTypesOrEmpty = (node: SdsClass | undefined): SdsType[] => {
return node?.parentTypeList?.parentTypes ?? [];
};

export const placeholdersOrEmpty = (node: SdsBlock | undefined): SdsPlaceholder[] => {
return stream(statementsOrEmpty(node))
.filter(isSdsAssignment)
Expand Down
3 changes: 3 additions & 0 deletions src/language/safe-ds-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { SafeDsClasses } from './builtins/safe-ds-classes.js';
import { SafeDsPackageManager } from './workspace/safe-ds-package-manager.js';
import { SafeDsNodeMapper } from './helpers/safe-ds-node-mapper.js';
import { SafeDsAnnotations } from './builtins/safe-ds-annotations.js';
import { SafeDsClassHierarchy } from './typing/safe-ds-class-hierarchy.js';

/**
* Declaration of custom services - add your own service classes here.
Expand All @@ -34,6 +35,7 @@ export type SafeDsAddedServices = {
NodeMapper: SafeDsNodeMapper;
};
types: {
ClassHierarchy: SafeDsClassHierarchy;
TypeComputer: SafeDsTypeComputer;
};
workspace: {
Expand Down Expand Up @@ -71,6 +73,7 @@ export const SafeDsModule: Module<SafeDsServices, PartialLangiumServices & SafeD
ScopeProvider: (services) => new SafeDsScopeProvider(services),
},
types: {
ClassHierarchy: (services) => new SafeDsClassHierarchy(services),
TypeComputer: (services) => new SafeDsTypeComputer(services),
},
workspace: {
Expand Down
60 changes: 60 additions & 0 deletions src/language/typing/safe-ds-class-hierarchy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { SafeDsServices } from '../safe-ds-module.js';
import { SafeDsClasses } from '../builtins/safe-ds-classes.js';
import { SdsClass } from '../generated/ast.js';
import { stream, Stream } from 'langium';
import { parentTypesOrEmpty } from '../helpers/nodeProperties.js';
import { SafeDsTypeComputer } from './safe-ds-type-computer.js';
import { ClassType } from './model.js';

export class SafeDsClassHierarchy {
private readonly builtinClasses: SafeDsClasses;
private readonly typeComputer: SafeDsTypeComputer;

constructor(services: SafeDsServices) {
this.builtinClasses = services.builtins.Classes;
this.typeComputer = services.types.TypeComputer;
}

/**
* Returns a stream of all superclasses of the given class. The class itself is not included in the stream unless
* there is a cycle in the inheritance hierarchy. Direct ancestors are returned first, followed by their ancestors
* and so on.
*/
streamSuperclasses(node: SdsClass | undefined): Stream<SdsClass> {
if (!node) {
return stream();
}

const capturedThis = this;
const generator = function* () {
const visited = new Set<SdsClass>();
let current = capturedThis.parentClassOrUndefined(node);
while (current && !visited.has(current)) {
yield current;
visited.add(current);
current = capturedThis.parentClassOrUndefined(current);
}

const anyClass = capturedThis.builtinClasses.Any;
if (anyClass && node !== anyClass && !visited.has(anyClass)) {
yield anyClass;
}
};

return stream(generator());
}

/**
* Returns the parent class of the given class, or undefined if there is no parent class. Only the first parent
* type is considered, i.e. multiple inheritance is not supported.
*/
private parentClassOrUndefined(node: SdsClass | undefined): SdsClass | undefined {
const [firstParentType] = parentTypesOrEmpty(node);
const computedType = this.typeComputer.computeType(firstParentType);
if (computedType instanceof ClassType) {
return computedType.sdsClass;
}

return undefined;
}
}
23 changes: 13 additions & 10 deletions src/language/typing/safe-ds-type-computer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,22 @@ import {

export class SafeDsTypeComputer {
private readonly astNodeLocator: AstNodeLocator;
private readonly coreClasses: SafeDsClasses;
private readonly builtinClasses: SafeDsClasses;
private readonly nodeMapper: SafeDsNodeMapper;

private readonly typeCache: WorkspaceCache<string, Type>;

constructor(services: SafeDsServices) {
this.astNodeLocator = services.workspace.AstNodeLocator;
this.coreClasses = services.builtins.Classes;
this.builtinClasses = services.builtins.Classes;
this.nodeMapper = services.helpers.NodeMapper;

this.typeCache = new WorkspaceCache(services.shared);
}

/**
* Computes the type of the given node.
*/
computeType(node: AstNode | undefined): Type {
if (!node) {
return UnknownType;
Expand Down Expand Up @@ -507,35 +510,35 @@ export class SafeDsTypeComputer {
// -----------------------------------------------------------------------------------------------------------------

private AnyOrNull(): Type {
return this.createCoreType(this.coreClasses.Any, true);
return this.createCoreType(this.builtinClasses.Any, true);
}

private Boolean(): Type {
return this.createCoreType(this.coreClasses.Boolean);
return this.createCoreType(this.builtinClasses.Boolean);
}

private Float(): Type {
return this.createCoreType(this.coreClasses.Float);
return this.createCoreType(this.builtinClasses.Float);
}

private Int(): Type {
return this.createCoreType(this.coreClasses.Int);
return this.createCoreType(this.builtinClasses.Int);
}

private List(): Type {
return this.createCoreType(this.coreClasses.List);
return this.createCoreType(this.builtinClasses.List);
}

private Map(): Type {
return this.createCoreType(this.coreClasses.Map);
return this.createCoreType(this.builtinClasses.Map);
}

private NothingOrNull(): Type {
return this.createCoreType(this.coreClasses.Nothing, true);
return this.createCoreType(this.builtinClasses.Nothing, true);
}

private String(): Type {
return this.createCoreType(this.coreClasses.String);
return this.createCoreType(this.builtinClasses.String);
}

private createCoreType(coreClass: SdsClass | undefined, isNullable: boolean = false): Type {
Expand Down
55 changes: 55 additions & 0 deletions src/language/validation/inheritance.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { ValidationAcceptor } from 'langium';
import { SdsClass } from '../generated/ast.js';
import { parentTypesOrEmpty } from '../helpers/nodeProperties.js';
import { isEmpty } from 'radash';
import { SafeDsServices } from '../safe-ds-module.js';
import { ClassType, UnknownType } from '../typing/model.js';

export const CODE_INHERITANCE_CYCLE = 'inheritance/cycle';
export const CODE_INHERITANCE_MULTIPLE_INHERITANCE = 'inheritance/multiple-inheritance';
export const CODE_INHERITANCE_NOT_A_CLASS = 'inheritance/not-a-class';

export const classMustOnlyInheritASingleClass = (services: SafeDsServices) => {
const typeComputer = services.types.TypeComputer;
const computeType = typeComputer.computeType.bind(typeComputer);

return (node: SdsClass, accept: ValidationAcceptor): void => {
const parentTypes = parentTypesOrEmpty(node);
if (isEmpty(parentTypes)) {
return;
}

const [firstParentType, ...otherParentTypes] = parentTypes;

// First parent type must be a class
const computedType = computeType(firstParentType);
if (computedType !== UnknownType && !(computedType instanceof ClassType)) {
accept('error', 'A class must only inherit classes.', {
node: firstParentType,
code: CODE_INHERITANCE_NOT_A_CLASS,
});
}

// Must have only one parent type
for (const parentType of otherParentTypes) {
accept('error', 'Multiple inheritance is not supported. Only the first parent type will be considered.', {
node: parentType,
code: CODE_INHERITANCE_MULTIPLE_INHERITANCE,
});
}
};
};

export const classMustNotInheritItself = (services: SafeDsServices) => {
const classHierarchy = services.types.ClassHierarchy;

return (node: SdsClass, accept: ValidationAcceptor): void => {
const superClasses = classHierarchy.streamSuperclasses(node);
if (superClasses.includes(node)) {
accept('error', 'A class must not directly or indirectly be a subtype of itself.', {
node: parentTypesOrEmpty(node)[0],
code: CODE_INHERITANCE_CYCLE,
});
}
};
};
9 changes: 7 additions & 2 deletions src/language/validation/safe-ds-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import {
import { placeholderShouldBeUsed, placeholdersMustNotBeAnAlias } from './other/declarations/placeholders.js';
import { segmentParameterShouldBeUsed, segmentResultMustBeAssignedExactlyOnce } from './other/declarations/segments.js';
import { lambdaParameterMustNotHaveConstModifier } from './other/expressions/lambdas.js';
import { indexedAccessesShouldBeUsedWithCaution } from './experimentalLanguageFeature.js';
import { indexedAccessesShouldBeUsedWithCaution } from './experimentalLanguageFeatures.js';
import { requiredParameterMustNotBeExpert } from './builtins/expert.js';
import {
callableTypeParametersMustNotBeAnnotated,
Expand All @@ -86,6 +86,7 @@ import {
namedTypeMustNotSetTypeParameterMultipleTimes,
namedTypeTypeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments,
} from './other/types/namedTypes.js';
import { classMustNotInheritItself, classMustOnlyInheritASingleClass } from './inheritance.js';

/**
* Register custom validation checks.
Expand Down Expand Up @@ -124,7 +125,11 @@ export const registerValidationChecks = function (services: SafeDsServices) {
callableTypeParameterMustNotHaveConstModifier,
callableTypeResultsMustNotBeAnnotated,
],
SdsClass: [classMustContainUniqueNames],
SdsClass: [
classMustContainUniqueNames,
classMustOnlyInheritASingleClass(services),
classMustNotInheritItself(services),
],
SdsClassBody: [classBodyShouldNotBeEmpty],
SdsConstraintList: [constraintListShouldNotBeEmpty],
SdsDeclaration: [nameMustNotStartWithBlockLambdaPrefix, nameShouldHaveCorrectCasing],
Expand Down
2 changes: 1 addition & 1 deletion tests/language/helpers/stringUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('pluralize', () => {
singular: 'apple',
expected: 'apples',
},
])('should return the singular or plural form based on the count', ({ count, singular, plural, expected }) => {
])('should return the singular or plural form based on the count (%#)', ({ count, singular, plural, expected }) => {
expect(pluralize(count, singular, plural)).toBe(expected);
});
});
88 changes: 88 additions & 0 deletions tests/language/typing/safe-ds-class-hierarchy.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { createSafeDsServices } from '../../../src/language/safe-ds-module.js';
import { NodeFileSystem } from 'langium/node';
import { clearDocuments } from 'langium/test';
import { isSdsClass, SdsClass } from '../../../src/language/generated/ast.js';
import { getNodeOfType } from '../../helpers/nodeFinder.js';

const services = createSafeDsServices(NodeFileSystem).SafeDs;
const classHierarchy = services.types.ClassHierarchy;

describe('SafeDsClassHierarchy', async () => {
beforeEach(async () => {
// Load the builtin library
await services.shared.workspace.WorkspaceManager.initializeWorkspace([]);
});

afterEach(async () => {
await clearDocuments(services);
});

describe('streamSuperclasses', () => {
const superclassNames = (node: SdsClass | undefined) =>
classHierarchy
.streamSuperclasses(node)
.map((clazz) => clazz.name)
.toArray();

it('should return an empty stream if passed undefined', () => {
expect(superclassNames(undefined)).toStrictEqual([]);
});

const testCases = [
{
testName: 'should return "Any" if the class has no parent types',
code: `
class A
`,
expected: ['Any'],
},
{
testName: 'should return "Any" if the first parent type is not a class',
code: `
class A sub E
enum E
`,
expected: ['Any'],
},
{
testName: 'should return the superclasses of a class (no cycle, implicit any)',
code: `
class A sub B
class B
`,
expected: ['B', 'Any'],
},
{
testName: 'should return the superclasses of a class (no cycle, explicit any)',
code: `
class A sub Any
`,
expected: ['Any'],
},
{
testName: 'should return the superclasses of a class (cycle)',
code: `
class A sub B
class B sub C
class C sub A
`,
expected: ['B', 'C', 'A', 'Any'],
},
{
testName: 'should only consider the first parent type',
code: `
class A sub B, C
class B
class C
`,
expected: ['B', 'Any'],
},
];

it.each(testCases)('$testName', async ({ code, expected }) => {
const firstClass = await getNodeOfType(services, code, isSdsClass);
expect(superclassNames(firstClass)).toStrictEqual(expected);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package tests.validation.inheritance.mustBeAcyclic

// $TEST$ error "A class must not directly or indirectly be a subtype of itself."
class MyClass1 sub »MyClass3«
// $TEST$ error "A class must not directly or indirectly be a subtype of itself."
class MyClass2 sub »MyClass1«
// $TEST$ error "A class must not directly or indirectly be a subtype of itself."
class MyClass3 sub »MyClass2«

class MyClass4
// $TEST$ no error "A class must not directly or indirectly be a subtype of itself."
class MyClass5 sub »MyClass4«

// $TEST$ no error "A class must not directly or indirectly be a subtype of itself."
class MyClass6 sub »MyClass7«
// $TEST$ no error "A class must not directly or indirectly be a subtype of itself."
class MyClass7 sub Any, »MyClass6«

// $TEST$ no error "A class must not directly or indirectly be a subtype of itself."
class MyClass8 sub »Unresolved«
// $TEST$ no error "A class must not directly or indirectly be a subtype of itself."
class MyClass9 sub »MyClass8«
Loading