Skip to content

Commit

Permalink
limit notification annotation suppport only for node claseses
Browse files Browse the repository at this point in the history
  • Loading branch information
daci committed Feb 15, 2024
1 parent 597dd7f commit 3203972
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 2 deletions.
8 changes: 7 additions & 1 deletion src/lib/utils/Diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -458,4 +458,10 @@ export function onNotificationConstructorError() {
};
}


export function onNotificationNotSupported() {
return {
message: `@onNotification is only supported on methods in a node class`,
code: `MSTO${1071}`,
severity: DiagnosticSeverity.Error
};
}
128 changes: 128 additions & 0 deletions src/plugin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4000,10 +4000,30 @@ describe('MaestroPlugin', () => {
end function
`);
});

it('fails validation if onNotification annotation is added on a method class that is not a node', async () => {
plugin.maestroConfig.allowNotificationAnnotations = true;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
class Comp
function initialize()
end function
private json
@onNotification("test")
function classMethod(arg1 as mc.notification)
end function
end class
`);
program.validate();
await builder.transpile();
let d = builder.getDiagnostics().filter((d) => d.severity === DiagnosticSeverity.Error && d.code !== 1044);
expect(d[0].code).to.equal('MSTO1071');
});
it('fails validation if onNotification annotation are disabled', async () => {
plugin.maestroConfig.allowNotificationAnnotations = false;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
function initialize()
end function
Expand All @@ -4023,6 +4043,7 @@ describe('MaestroPlugin', () => {
plugin.maestroConfig.allowNotificationAnnotations = true;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
function initialize()
end function
Expand All @@ -4042,6 +4063,7 @@ describe('MaestroPlugin', () => {
plugin.maestroConfig.allowNotificationAnnotations = true;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
function initialize()
end function
Expand All @@ -4051,6 +4073,8 @@ describe('MaestroPlugin', () => {
end function
end class
`);


program.validate();
await builder.transpile();
let d = builder.getDiagnostics().filter((d) => d.severity === DiagnosticSeverity.Error);
Expand All @@ -4061,6 +4085,7 @@ describe('MaestroPlugin', () => {
plugin.maestroConfig.allowNotificationAnnotations = true;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
private json
@onNotification("test")
Expand All @@ -4078,6 +4103,7 @@ describe('MaestroPlugin', () => {
plugin.maestroConfig.allowNotificationAnnotations = true;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
private json
function initialize()
Expand All @@ -4090,6 +4116,57 @@ describe('MaestroPlugin', () => {
program.validate();
await builder.transpile();
//ignore diagnostics - need to import core
expect(
getContents('components/maestro/generated/Comp.xml')
).to.eql(undent`
<?xml version="1.0" encoding="UTF-8" ?>
<component name="Comp" extends="Group">
<interface>
<function name="initialize" />
<function name="classMethod" />
</interface>
<script type="text/brightscript" uri="pkg:/components/maestro/generated/Comp.brs" />
<script type="text/brightscript" uri="pkg:/source/comp.brs" />
<script type="text/brightscript" uri="pkg:/source/bslib.brs" />
<children />
</component>
`);

expect(
getContents('components/maestro/generated/Comp.brs')
).to.eql(undent`
'import "pkg:/source/comp.bs"
function init()
instance = __Comp_builder()
instance.delete("top")
instance.delete("global")
top = m.top
m.append(instance)
m.__isVMCreated = true
m.new()
m.top = top
m_wireUpObservers()
end function
function m_wireUpObservers()
end function
function __m_setTopField(field, value)
if m.top.doesExist(field)
m.top[field] = value
end if
return value
end function
function initialize(dummy = invalid)
return m.initialize()
end function
function classMethod(notification = invalid)
return m.classMethod(notification)
end function
`);
expect(
getContents('source/comp.brs')
).to.eql(undent`
Expand Down Expand Up @@ -4118,6 +4195,7 @@ describe('MaestroPlugin', () => {
plugin.maestroConfig.allowNotificationAnnotations = true;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
private json
@onNotification("test")
Expand All @@ -4134,6 +4212,52 @@ describe('MaestroPlugin', () => {

let d = builder.getDiagnostics().filter((d) => d.severity === DiagnosticSeverity.Error && d.code !== 1044);
expect(d[0].code).to.equal('MSTO1070');
expect(
getContents('components/maestro/generated/Comp.xml')
).to.eql(undent`
<?xml version="1.0" encoding="UTF-8" ?>
<component name="Comp" extends="Group">
<interface>
<function name="classMethod" />
</interface>
<script type="text/brightscript" uri="pkg:/components/maestro/generated/Comp.brs" />
<script type="text/brightscript" uri="pkg:/source/comp.brs" />
<script type="text/brightscript" uri="pkg:/source/bslib.brs" />
<children />
</component>
`);

expect(
getContents('components/maestro/generated/Comp.brs')
).to.eql(undent`
'import "pkg:/source/comp.bs"
function init()
instance = __Comp_builder()
instance.delete("top")
instance.delete("global")
top = m.top
m.append(instance)
m.__isVMCreated = true
m.new()
m.top = top
m_wireUpObservers()
end function
function m_wireUpObservers()
end function
function __m_setTopField(field, value)
if m.top.doesExist(field)
m.top[field] = value
end if
return value
end function
function classMethod(notification = invalid)
return m.classMethod(notification)
end function
`);
expect(
getContents('source/comp.brs')
).to.eql(undent`
Expand All @@ -4159,6 +4283,7 @@ describe('MaestroPlugin', () => {
plugin.maestroConfig.allowNotificationAnnotations = true;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
private json
sub new()
Expand Down Expand Up @@ -4339,6 +4464,7 @@ describe('MaestroPlugin', () => {
plugin.afterProgramCreate(program);
plugin.maestroConfig.allowNotificationAnnotations = true;
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
private json
@onNotification()
Expand All @@ -4358,6 +4484,7 @@ describe('MaestroPlugin', () => {
plugin.maestroConfig.allowNotificationAnnotations = true;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
@onNotification("test")
private json
Expand All @@ -4377,6 +4504,7 @@ describe('MaestroPlugin', () => {
plugin.maestroConfig.allowNotificationAnnotations = false;
plugin.afterProgramCreate(program);
program.setFile('source/comp.bs', `
@node("Comp", "Group")
class Comp
private json
@onNotification("test")
Expand Down
20 changes: 19 additions & 1 deletion src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import ReflectionUtil from './lib/reflection/ReflectionUtil';
import { FileFactory } from './lib/utils/FileFactory';
import NodeClassUtil from './lib/node-classes/NodeClassUtil';
import { RawCodeStatement, RawCodeExpression } from './lib/utils/RawCodeStatement';
import { addClassFieldsNotFoundOnSetOrGet, addIOCNoTypeSupplied, addIOCWrongArgs, noCallsInAsXXXAllowed, functionNotImported, IOCClassNotInScope, namespaceNotImported, noPathForInject, noPathForIOCSync, unknownClassMethod, unknownConstructorMethod, unknownSuperClass, unknownType, wrongConstructorArgs, wrongMethodArgs, observeRequiresFirstArgumentIsField, observeRequiresFirstArgumentIsNotM, observeFunctionNameNotFound, observeFunctionNameWrongArgs, addWrongAnnotation, noNameForNotification, onNotificationFieldError, notificationAnnotationDisabled,onNotificationWrongParameter, onNotificationConstructorError } from './lib/utils/Diagnostics';
import { addClassFieldsNotFoundOnSetOrGet, addIOCNoTypeSupplied, addIOCWrongArgs, noCallsInAsXXXAllowed, functionNotImported, IOCClassNotInScope, namespaceNotImported, noPathForInject, noPathForIOCSync, unknownClassMethod, unknownConstructorMethod, unknownSuperClass, unknownType, wrongConstructorArgs, wrongMethodArgs, observeRequiresFirstArgumentIsField, observeRequiresFirstArgumentIsNotM, observeFunctionNameNotFound, observeFunctionNameWrongArgs, addWrongAnnotation, noNameForNotification, onNotificationFieldError, notificationAnnotationDisabled,onNotificationWrongParameter, onNotificationConstructorError, onNotificationNotSupported } from './lib/utils/Diagnostics';
import { getAllAnnotations, getAllFields, defaultAnnotations } from './lib/utils/Utils';
import { getSGMembersLookup } from './SGApi';
import { DynamicType } from 'brighterscript/dist/types/DynamicType';
Expand Down Expand Up @@ -288,6 +288,24 @@ export class MaestroPlugin implements CompilerPlugin {
if (!annotation) {
continue;
}
let classAnnotations = this.findAnnotations(method.parent) || [];
if (!classAnnotations.find((a) => a.name.toLowerCase() === 'node' || a.name.toLowerCase() === 'nodeclass')) {
file.addDiagnostics([{
...onNotificationNotSupported(),
range: cs.range,
file: file
}]);
continue
}
// let isNodeClass = (method.parent.annotations|| []).find((a) => a.name.toLowerCase() === 'node' || a.name.toLowerCase() === 'nodeclass' ;
// if (!isNodeClass) {
// file.addDiagnostics([{
// ...onNotificationNotSupported(),
// range: cs.range,
// file: file
// }]);
// continue
// }
if (method.name.text.toLowerCase() == "new") {
file.addDiagnostics([{
...onNotificationConstructorError(),
Expand Down

0 comments on commit 3203972

Please sign in to comment.