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

Remove FunctionScope and improve type tracking #258

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a24b93f
Add `for each` variable to completions
TwitchBronBron Dec 18, 2020
e4d01fa
ForEachStatement's `item` is now VaribleExpression
TwitchBronBron Dec 18, 2020
fc2100f
Fix ts build issue.
TwitchBronBron Dec 18, 2020
04d7b72
fix broken tests
TwitchBronBron Dec 18, 2020
91a7d21
Merge branch 'master' into for-each-var-completions
TwitchBronBron Dec 19, 2020
0e4589e
fix broken test.
TwitchBronBron Dec 19, 2020
948400c
Collection function variables in _references
TwitchBronBron Dec 19, 2020
4aa2f0d
Removed FunctionScope in favor of func expressions
TwitchBronBron Dec 21, 2020
2ab29ef
Merge branch 'master' into for-each-var-completions
TwitchBronBron Jan 13, 2021
62a6103
Merge branch 'master' of https://github.com/rokucommunity/brighterscr…
TwitchBronBron Jan 13, 2021
ea4f72e
lint and interface fixes
TwitchBronBron Jan 13, 2021
5613304
Repopulate `references.localVars` during `findReferences`
TwitchBronBron Jan 13, 2021
b1a93ce
Merge branch 'master' into for-each-var-completions
TwitchBronBron Jan 15, 2021
f916694
Handle sub with explicit return type
TwitchBronBron Jan 15, 2021
73c432c
Fix `findReferences` for local vars
TwitchBronBron Jan 15, 2021
03c99a8
Merge branch 'master' into for-each-var-completions
TwitchBronBron Jan 30, 2021
f2dc8f0
Merge branch 'master' into for-each-var-completions
TwitchBronBron Jan 30, 2021
b47b3a5
Normalize line endings for Scope.ts
TwitchBronBron Jan 30, 2021
eb6dad6
Merge branch 'master' into for-each-var-completions
TwitchBronBron Mar 16, 2021
c2a825b
Merge branch 'master' of https://github.com/rokucommunity/brighterscr…
TwitchBronBron Mar 16, 2021
54c356f
Merge branch 'master' into for-each-var-completions
TwitchBronBron Mar 16, 2021
60284a4
Merge branch 'master' of https://github.com/rokucommunity/brighterscr…
TwitchBronBron Mar 30, 2021
8fe2bf7
Feature/symbol table (#380)
markwpearce Apr 1, 2021
65c0d92
Feature/scope tracked symbol tables (#395)
markwpearce Apr 23, 2021
db329b0
Merge branch 'master' of https://github.com/rokucommunity/brighterscr…
TwitchBronBron Apr 23, 2021
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
40 changes: 0 additions & 40 deletions src/FunctionScope.spec.ts

This file was deleted.

53 changes: 0 additions & 53 deletions src/FunctionScope.ts

This file was deleted.

28 changes: 28 additions & 0 deletions src/Program.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,34 @@ describe('Program', () => {
});

describe('getCompletions', () => {
it('includes `for each` variable', async () => {
await program.addOrReplaceFile('source/main.brs', `
sub main()
items = [1, 2, 3]
for each thing in items
t =
end for
end for
end sub
`);
await program.validate();
let completions = (await program.getCompletions(`${rootDir}/source/main.brs`, Position.create(4, 28))).map(x => x.label);
expect(completions).to.include('thing');
});

it('includes `for` variable', async () => {
await program.addOrReplaceFile('source/main.brs', `
sub main()
for i = 0 to 10
t =
end for
end sub
`);
await program.validate();
let completions = (await program.getCompletions(`${rootDir}/source/main.brs`, Position.create(3, 28))).map(x => x.label);
expect(completions).to.include('i');
});

it('should include first-level namespace names for brighterscript files', async () => {
await program.addOrReplaceFile('source/main.bs', `
namespace NameA.NameB.NameC
Expand Down
41 changes: 20 additions & 21 deletions src/Scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -612,63 +612,61 @@ export class Scope {
}

/**
* Detect local variables (function scope) that have the same name as scope calls
* Detect local variables (vars declared within a function expression) that have the same name as scope calls
* @param file
* @param callableContainerMap
*/
private diagnosticDetectShadowedLocalVars(file: BscFile, callableContainerMap: CallableContainerMap) {
const classMap = this.getClassMap();
//loop through every function scope
for (let scope of file.functionScopes) {
//every var declaration in this function scope
for (let varDeclaration of scope.variableDeclarations) {
const varName = varDeclaration.name;
const lowerVarName = varName.toLowerCase();

for (let functionExpression of file.parser.references.functionExpressions) {
const localVars = file.parser.references.localVars.get(functionExpression);
//every var declaration in this function expression
for (let localVar of localVars) {
//if the var is a function
if (isFunctionType(varDeclaration.type)) {
if (isFunctionType(localVar.type)) {
//local var function with same name as stdlib function
if (
//has same name as stdlib
globalCallableMap.has(lowerVarName)
globalCallableMap.has(localVar.lowerName)
) {
this.diagnostics.push({
...DiagnosticMessages.localVarFunctionShadowsParentFunction('stdlib'),
range: varDeclaration.nameRange,
range: localVar.nameToken.range,
file: file
});

//this check needs to come after the stdlib one, because the stdlib functions are included
//in the scope function list
} else if (
//has same name as scope function
callableContainerMap.has(lowerVarName)
callableContainerMap.has(localVar.lowerName)
) {
this.diagnostics.push({
...DiagnosticMessages.localVarFunctionShadowsParentFunction('scope'),
range: varDeclaration.nameRange,
range: localVar.nameToken.range,
file: file
});
}

//var is not a function
} else if (
//is NOT a callable from stdlib (because non-function local vars can have same name as stdlib names)
!globalCallableMap.has(lowerVarName)
!globalCallableMap.has(localVar.lowerName)
) {

//is same name as a callable
if (callableContainerMap.has(lowerVarName)) {
if (callableContainerMap.has(localVar.lowerName)) {
this.diagnostics.push({
...DiagnosticMessages.localVarShadowedByScopedFunction(),
range: varDeclaration.nameRange,
range: localVar.nameToken.range,
file: file
});
//has the same name as an in-scope class
} else if (classMap.has(lowerVarName)) {
} else if (classMap.has(localVar.lowerName)) {
this.diagnostics.push({
...DiagnosticMessages.localVarSameNameAsClass(classMap.get(lowerVarName).getName(ParseMode.BrighterScript)),
range: varDeclaration.nameRange,
...DiagnosticMessages.localVarSameNameAsClass(classMap.get(localVar.lowerName).getName(ParseMode.BrighterScript)),
range: localVar.nameToken.range,
file: file
});
}
Expand All @@ -683,6 +681,7 @@ export class Scope {
* @param callablesByLowerName
*/
private diagnosticDetectCallsToUnknownFunctions(file: BscFile, callablesByLowerName: CallableContainerMap) {

//validate all expression calls
for (let expCall of file.functionCalls) {
const lowerName = expCall.name.toLowerCase();
Expand All @@ -692,11 +691,11 @@ export class Scope {
continue;
}

//get the local scope for this expression
let scope = file.getFunctionScopeAtPosition(expCall.nameRange.start);
//find a local variable with this name
const localVar = file.getLocalVarsAtPosition(expCall.nameRange.start).find(x => x.lowerName === lowerName);

//if we don't already have a variable with this name.
if (!scope?.getVariableByName(lowerName)) {
if (!localVar) {
let callablesWithThisName = callablesByLowerName.get(lowerName);

//use the first item from callablesByLowerName, because if there are more, that's a separate error
Expand Down
4 changes: 2 additions & 2 deletions src/astUtils/reflection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('reflection', () => {
const expr = createStringLiteral('', range);
const token = createToken(TokenKind.StringLiteral, '', range);
const body = new Body([]);
const assignment = new AssignmentStatement(undefined, ident, expr, undefined);
const assignment = new AssignmentStatement(ident, token, expr, undefined);
const block = new Block([], range);
const expression = new ExpressionStatement(expr);
const comment = new CommentStatement([token]);
Expand All @@ -44,7 +44,7 @@ describe('reflection', () => {
const ends = new EndStatement({ end: token });
const stop = new StopStatement({ stop: token });
const fors = new ForStatement(token, assignment, token, expr, block, token, token, expr);
const foreach = new ForEachStatement({ forEach: token, in: token, endFor: token }, token, expr, block);
const foreach = new ForEachStatement(token, ident, token, expr, block, token);
const whiles = new WhileStatement({ while: token, endWhile: token }, expr, block);
const dottedSet = new DottedSetStatement(expr, ident, expr);
const indexedSet = new IndexedSetStatement(expr, expr, expr, token, token);
Expand Down
Loading