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

Allow nested namespaces #708

Merged
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
29 changes: 29 additions & 0 deletions docs/namespaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,32 @@ sub Vertibrates_Birds_Quack() ' this will result in a compile error.
end sub
```
</details>

## Nesting namespaces

In the above examples, nested namespaces are formed by specifying a _dotted identifier_ (`NameA.NameB`). You can also declare namespaces within other namespaces to form a dotted namespace identifier which contains its parent namespaces.

```BrighterScript
namespace Vertibrates
namespace Birds
sub Quack()
end sub
end namespace

namespace Reptiles
sub Hiss()
end sub
end
end namespace
```

<details>
<summary>View the transpiled BrightScript code</summary>

```BrightScript
sub Vertibrates_Birds_Quack()
end sub
sub Vertibrates_Reptiles_Hiss()
end sub
```
</details>
5 changes: 5 additions & 0 deletions src/DiagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,11 @@ export let DiagnosticMessages = {
message: `Continue statement must be contained within a loop statement`,
code: 1135,
severity: DiagnosticSeverity.Error
}),
keywordMustBeDeclaredAtNamespaceLevel: (keyword: string) => ({
message: `${keyword} must be declared at the root level or within a namespace`,
code: 1136,
severity: DiagnosticSeverity.Error
})
};

Expand Down
1 change: 0 additions & 1 deletion src/Program.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,6 @@ describe('Program', () => {
class MyClassA
end class
end namespace
namespace NameA.NameB
namespace NameA.NoClassA
end namespace
namespace NameA.NoClassB
Expand Down
31 changes: 31 additions & 0 deletions src/bscPlugin/validation/BrsFileValidator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { expect } from 'chai';
import type { BrsFile } from '../../files/BrsFile';
import type { AALiteralExpression, DottedGetExpression } from '../../parser/Expression';
import type { ClassStatement, FunctionStatement, NamespaceStatement, PrintStatement } from '../../parser/Statement';
import { DiagnosticMessages } from '../../DiagnosticMessages';
import { expectDiagnostics, expectZeroDiagnostics } from '../../testHelpers.spec';
import { Program } from '../../Program';

describe('BrsFileValidator', () => {
Expand Down Expand Up @@ -54,4 +56,33 @@ describe('BrsFileValidator', () => {
const alpha = bravo.obj as DottedGetExpression;
expect(alpha.parent).to.equal(bravo);
});

describe('namespace validation', () => {
it('succeeds if namespaces are defined inside other namespaces', () => {
program.setFile<BrsFile>('source/main.bs', `
namespace alpha
' random comment
namespace bravo
' random comment
sub main()
end sub
end namespace
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});
it('fails if namespaces are defined inside a function', () => {
program.setFile<BrsFile>('source/main.bs', `
function f()
namespace alpha
end namespace
end function
`);
program.validate();
expectDiagnostics(program, [
DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('namespace')
]);
});
});
});
22 changes: 20 additions & 2 deletions src/bscPlugin/validation/BrsFileValidator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isClassStatement, isCommentStatement, isConstStatement, isDottedGetExpression, isEnumStatement, isForEachStatement, isForStatement, isFunctionStatement, isImportStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isNamespacedVariableNameExpression, isNamespaceStatement, isUnaryExpression, isWhileStatement } from '../../astUtils/reflection';
import { isBody, isClassStatement, isCommentStatement, isConstStatement, isDottedGetExpression, isEnumStatement, isForEachStatement, isForStatement, isFunctionStatement, isImportStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isNamespacedVariableNameExpression, isNamespaceStatement, isUnaryExpression, isWhileStatement } from '../../astUtils/reflection';
import { createVisitor, WalkMode } from '../../astUtils/visitors';
import { DiagnosticMessages } from '../../DiagnosticMessages';
import type { BrsFile } from '../../files/BrsFile';
Expand All @@ -7,7 +7,7 @@ import { TokenKind } from '../../lexer/TokenKind';
import type { AstNode, Expression } from '../../parser/AstNode';
import type { LiteralExpression } from '../../parser/Expression';
import { ParseMode } from '../../parser/Parser';
import type { ContinueStatement, EnumMemberStatement, EnumStatement, ForEachStatement, ForStatement, ImportStatement, LibraryStatement, WhileStatement } from '../../parser/Statement';
import type { ContinueStatement, EnumMemberStatement, EnumStatement, ForEachStatement, ForStatement, ImportStatement, LibraryStatement, NamespaceStatement, WhileStatement } from '../../parser/Statement';
import { DynamicType } from '../../types/DynamicType';
import util from '../../util';

Expand Down Expand Up @@ -101,6 +101,8 @@ export class BrsFileValidator {
node.parent.getSymbolTable()?.addSymbol(node.item.text, node.item.range, DynamicType.instance);
},
NamespaceStatement: (node) => {
this.validateNamespaceStatement(node);

node.parent.getSymbolTable().addSymbol(
node.name.split('.')[0],
node.nameExpression.range,
Expand Down Expand Up @@ -238,6 +240,22 @@ export class BrsFileValidator {
}
}

private validateNamespaceStatement(stmt: NamespaceStatement) {
let parentNode = stmt.parent;

while (parentNode) {
if (!isNamespaceStatement(parentNode) && !isBody(parentNode)) {
this.event.file.addDiagnostic({
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('namespace'),
range: stmt.range
});
break;
}

parentNode = parentNode.parent;
}
}

/**
* Find statements defined at the top level (or inside a namespace body) that are not allowed to be there
*/
Expand Down
24 changes: 16 additions & 8 deletions src/parser/Parser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,16 +365,24 @@ describe('parser', () => {
end sub
`, ParseMode.BrighterScript).diagnostics[0]?.message).not.to.exist;
});

describe('namespace', () => {
it('catches namespaces declared not at root level', () => {
expect(parse(`
sub main()
namespace Name.Space
it('allows namespaces declared inside other namespaces', () => {
const parser = parse(`
namespace Level1
namespace Level2.Level3
sub main()
end sub
end namespace
end sub
`, ParseMode.BrighterScript).diagnostics[0]?.message).to.equal(
DiagnosticMessages.keywordMustBeDeclaredAtRootLevel('namespace').message
);
end namespace
`, ParseMode.BrighterScript);
expectZeroDiagnostics(parser);
// We expect these names to be "as given" in this context, because we aren't
// evaluating a full program.
expect(parser.references.namespaceStatements.map(statement => statement.getName(ParseMode.BrighterScript))).to.deep.equal([
elliot-nelson marked this conversation as resolved.
Show resolved Hide resolved
'Level2.Level3',
'Level1'
]);
});
it('parses empty namespace', () => {
let { statements, diagnostics } =
Expand Down
7 changes: 1 addition & 6 deletions src/parser/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1313,12 +1313,6 @@ export class Parser {
this.warnIfNotBrighterScriptMode('namespace');
let keyword = this.advance();

if (!this.isAtRootLevel()) {
this.diagnostics.push({
...DiagnosticMessages.keywordMustBeDeclaredAtRootLevel('namespace'),
range: keyword.range
});
}
this.namespaceAndFunctionDepth++;

let name = this.getNamespacedVariableNameExpression();
Expand All @@ -1341,6 +1335,7 @@ export class Parser {
}

this.namespaceAndFunctionDepth--;

result.body = body;
result.endKeyword = endKeyword;
this._references.namespaceStatements.push(result);
Expand Down
19 changes: 19 additions & 0 deletions src/parser/Statement.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { NamespaceStatement } from './Statement';
import { Body, CommentStatement, EmptyStatement } from './Statement';
import { ParseMode, Parser } from './Parser';
import { WalkMode } from '../astUtils/visitors';
import { isNamespaceStatement } from '../astUtils/reflection';
import { CancellationTokenSource } from 'vscode-languageserver';
import { Program } from '../Program';
import { trim } from '../testHelpers.spec';
Expand Down Expand Up @@ -46,6 +47,24 @@ describe('Statement', () => {
expect(statement.getName(ParseMode.BrighterScript)).to.equal('NameA.NameB');
expect(statement.getName(ParseMode.BrightScript)).to.equal('NameA_NameB');
});

it('getName() works', () => {
program.setFile<BrsFile>('source/main.brs', `
namespace NameA
namespace NameB
sub main()
end sub
end namespace
end namespace
`);
program.validate();
let node = program.getFile<BrsFile>('source/main.brs').ast.findChild<NamespaceStatement>(isNamespaceStatement);
while (node.findChild(isNamespaceStatement)) {
node = node.findChild<NamespaceStatement>(isNamespaceStatement);
}
expect(node.getName(ParseMode.BrighterScript)).to.equal('NameA.NameB');
expect(node.getName(ParseMode.BrightScript)).to.equal('NameA_NameB');
});
});

describe('CommentStatement', () => {
Expand Down
12 changes: 10 additions & 2 deletions src/parser/Statement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,7 @@ export class LibraryStatement extends Statement implements TypedefProvider {
export class NamespaceStatement extends Statement implements TypedefProvider {
constructor(
public keyword: Token,
//this should technically only be a VariableExpression or DottedGetExpression, but that can be enforced elsewhere
// this should technically only be a VariableExpression or DottedGetExpression, but that can be enforced elsewhere
public nameExpression: NamespacedVariableNameExpression,
public body: Body,
public endKeyword: Token
Expand Down Expand Up @@ -1113,7 +1113,15 @@ export class NamespaceStatement extends Statement implements TypedefProvider {
}

public getName(parseMode: ParseMode) {
return this.nameExpression.getName(parseMode);
const parentNamespace = this.findAncestor<NamespaceStatement>(isNamespaceStatement);
let name = this.nameExpression.getName(parseMode);

if (parentNamespace) {
const sep = parseMode === ParseMode.BrighterScript ? '.' : '_';
name = parentNamespace.getName(parseMode) + sep + name;
}

return name;
}

transpile(state: BrsTranspileState) {
Expand Down