Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Catch circular class extend references
  • Loading branch information
TwitchBronBron committed Feb 18, 2021
commit 38f93918ac50b9000f97a9fc3cdf0a24beb43811
5 changes: 5 additions & 0 deletions src/DiagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,11 @@ export let DiagnosticMessages = {
message: `Missing expression(s) in 'dim' statement`,
code: 1121,
severity: DiagnosticSeverity.Error
}),
circularReferenceDetected: (items: string[]) => ({
message: `Circular reference detected between ${Array.isArray(items) ? items.join(' -> ') : ''}`,
code: 1122,
severity: DiagnosticSeverity.Error
})
};

Expand Down
40 changes: 40 additions & 0 deletions src/files/BrsFile.Class.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,46 @@ describe('BrsFile BrighterScript classes', () => {
);
});

it('detects direct circular extends', () => {
//direct
program.addOrReplaceFile('source/Direct.bs', `
class Parent extends Child
end class

class Child extends Parent
end class
`);
program.validate();
expect(
program.getDiagnostics().map(x => x.message).sort()
).to.eql([
DiagnosticMessages.circularReferenceDetected(['Child', 'Parent', 'Child']).message,
DiagnosticMessages.circularReferenceDetected(['Parent', 'Child', 'Parent']).message
]);
});

it('detects indirect circular extends', () => {
//direct
program.addOrReplaceFile('source/Direct.bs', `
class Parent extends Grandchild
end class

class Child extends Parent
end class

class Grandchild extends Child
end class
`);
program.validate();
expect(
program.getDiagnostics().map(x => x.message).sort()
).to.eql([
DiagnosticMessages.circularReferenceDetected(['Child', 'Parent', 'Grandchild', 'Child']).message,
DiagnosticMessages.circularReferenceDetected(['Grandchild', 'Child', 'Parent', 'Grandchild']).message,
DiagnosticMessages.circularReferenceDetected(['Parent', 'Grandchild', 'Child', 'Parent']).message
]);
});

it('detects duplicate member names', () => {
program.addOrReplaceFile({ src: `${rootDir}/source/main.bs`, dest: 'source/main.bs' }, `
class Animal
Expand Down
33 changes: 31 additions & 2 deletions src/validators/ClassValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import type { BrsFile } from '../files/BrsFile';
export class BsClassValidator {
private scope: Scope;
public diagnostics: BsDiagnostic[];
/**
* The key is the namespace-prefixed class name. (i.e. `NameA.NameB.SomeClass` or `CoolClass`
*/
private classes: Record<string, AugmentedClassStatement>;

public validate(scope: Scope) {
Expand All @@ -24,6 +27,7 @@ export class BsClassValidator {
this.findClasses();
this.findNamespaceNonNamespaceCollisions();
this.linkClassesWithParents();
this.detectCircularReferences();
this.validateMemberCollisions();
this.verifyChildConstructor();
this.verifyNewExpressions();
Expand Down Expand Up @@ -156,6 +160,31 @@ export class BsClassValidator {
}
}

private detectCircularReferences() {
for (let key in this.classes) {
let cls = this.classes[key];
const names = new Map<string, string>();
do {
const className = cls.getName(ParseMode.BrighterScript);
const lowerClassName = className.toLowerCase();
//if we've already seen this class name before, then we have a circular dependency
if (names.has(lowerClassName)) {
this.diagnostics.push({
...DiagnosticMessages.circularReferenceDetected([
...names.values(),
className
]),
file: cls.file,
range: cls.name.range
});
break;
}
names.set(lowerClassName, className);
cls = cls.parentClass;
} while (cls);
}
}

private validateMemberCollisions() {
for (let key in this.classes) {
let classStatement = this.classes[key];
Expand Down Expand Up @@ -270,7 +299,7 @@ export class BsClassValidator {
/**
* Get the closest member with the specified name (case-insensitive)
*/
private getAncestorMember(classStatement: AugmentedClassStatement, memberName: string) {
getAncestorMember(classStatement, memberName) {
let lowerMemberName = memberName.toLowerCase();
let ancestor = classStatement.parentClass;
while (ancestor) {
Expand All @@ -281,7 +310,7 @@ export class BsClassValidator {
classStatement: ancestor
};
}
ancestor = ancestor.parentClass;
ancestor = ancestor.parentClass !== ancestor ? ancestor.parentClass : null;
}
}

Expand Down