Formal support and tests for various Variable and name shadowing rules (

#1089)

* Changed shadowing logic to only add diagnostics when namespaces match

* WIP of adding and refining tests

* Fixed all tests

* use cached callablecontainermap, refactor detectShadowedLocalVar to use Ast walk

* Updated docs

* updated readme

---------

Co-authored-by: Bronley Plumb <bronley@gmail.com>

docs/readme.md

@@ -111,7 +111,10 @@ authStatus = user <> invalid ? "logged in" : "not logged in"
 
 ## [Union Types](union-types.md)
 ```brighterscript
-sub log(data as string or number)
+sub logData(data as string or number)
     print data.toStr()
 end sub
-```
+```
+
+## [Variable Shadowing](variable-shadowing.md)
+Name resolution rules for various types of shadowing.

docs/variable-shadowing.md

@@ -0,0 +1,104 @@
+# Variable Shadowing and Name Collisions in BrighterScript
+
+BrighterScript allows various kinds of [variable name shadowing](https://en.wikipedia.org/wiki/Variable_shadowing). In general, variables and types refer to items defined in the current scope/namespace if available.
+
+However, due to the limitations of BrightScript, there are some name collisions that lead to unpredictable behavior, and so they cause diagnostic errors.
+
+## Name Resolution Rules
+
+**1. Local variables CAN shadow names of global functions.**
+
+✅
+
+```brighterscipt
+sub test()
+    log = "value" ' identifier shadows global function log() - no error
+    upTime = 0 ' identifier shadows global function Uptime() - no error
+end sub
+```
+
+**2. Local variables CANNOT shadow names of functions or classes defined at the same scope.**
+
+❌
+
+```brighterscipt
+sub test()
+    pi = "Apple" ' identifier shadows function pi() - causes validation error
+    data = 1234 ' identifier shadows class Data - causes validation error
+end sub
+
+function pi() as float
+    return 3.14
+function
+
+class Data
+    value = {}
+end class
+```
+
+**3. Custom types and definitions (enums, classes, interfaces, functions, consts) CANNOT have the same name if they are in the same namespace.**
+
+❌
+
+```brighterscipt
+function someName()
+end function
+
+class SomeName  ' class shadows local function - causes validation error
+    sub foo()
+    end sub
+end class
+
+namespace alpha
+    class OtherName  ' class in namespace shadows function in same namespace - causes validation error
+        sub foo()
+        end sub
+    end class
+
+    function otherName()
+    end function
+end namespace
+```
+
+**4. Functions and classes outside of namespaces CANNOT shadow standard global functions (eg. `ParseJson`, `LCase`, etc.)**
+
+❌
+
+```brighterscipt
+class log  ' class shadows global function - causes validation error
+    sub foo()
+    end sub
+end class
+
+
+```
+
+**5. Definitions inside namespaces CAN shadow standard global functions, or functions at a difference namespace-level. In this way, the outer item is unavailable, and only the item defined at the current scope is accessible.**
+
+✅
+
+```brighterscipt
+class SomeName()
+end class
+
+namespace alpha
+    class SomeName  ' class in namespace shadows function in upper scope
+        sub foo()
+            print "foo"
+        end sub
+    end class
+
+    sub test()
+        myKlass = new SomeName() ' refers to alpha.SomeName
+        myKlass.foo() ' will print "foo"
+    end sub
+
+    sub log(data) ' function defined as alpha.log - this is ok
+        print "LOG: ";data
+    end sub
+
+    sub foo()
+        log("Hello world") ' refers to alpha.log - will print "LOG: Hello world"
+    end sub
+end namespace
+```

src/AstValidationSegmenter.ts

@@ -1,11 +1,11 @@
 import type { DottedGetExpression, TypeExpression, VariableExpression } from './parser/Expression';
-import { SymbolTypeFlag } from './SymbolTableFlag';
 import { isBody, isClassStatement, isInterfaceStatement, isNamespaceStatement, isVariableExpression } from './astUtils/reflection';
 import { ChildrenSkipper, WalkMode, createVisitor } from './astUtils/visitors';
 import type { GetTypeOptions, TypeChainEntry } from './interfaces';
 import type { AstNode } from './parser/AstNode';
 import { util } from './util';
 import type { NamespaceStatement } from './parser/Statement';
+import { SymbolTypeFlag } from './SymbolTypeFlag';
 
 // eslint-disable-next-line no-bitwise
 export const InsideSegmentWalkMode = WalkMode.visitStatements | WalkMode.visitExpressions | WalkMode.recurseChildFunctions;

src/DiagnosticMessages.ts

@@ -3,7 +3,7 @@ import { DiagnosticSeverity } from 'vscode-languageserver';
 import type { BsDiagnostic, TypeCompatibilityData } from './interfaces';
 import { TokenKind } from './lexer/TokenKind';
 import util from './util';
-import { SymbolTypeFlag } from './SymbolTableFlag';
+import { SymbolTypeFlag } from './SymbolTypeFlag';
 
 /**
  * An object that keeps track of all possible error messages.

src/Program.spec.ts

@@ -21,7 +21,7 @@ import * as path from 'path';
 import type { SinonSpy } from 'sinon';
 import { createSandbox } from 'sinon';
 import type { AfterFileAddEvent, AfterFileRemoveEvent, AfterProvideFileEvent, BeforeFileAddEvent, BeforeFileRemoveEvent, BeforeProvideFileEvent, CompilerPlugin, ProvideFileEvent } from './interfaces';
-import { SymbolTypeFlag } from './SymbolTableFlag';
+import { SymbolTypeFlag } from './SymbolTypeFlag';
 import { StringType } from './types/StringType';
 import { TypedFunctionType } from './types/TypedFunctionType';
 import { DynamicType } from './types/DynamicType';

src/Program.ts

@@ -28,7 +28,7 @@ import { SignatureHelpUtil } from './bscPlugin/SignatureHelpUtil';
 import { DiagnosticSeverityAdjuster } from './DiagnosticSeverityAdjuster';
 import { IntegerType } from './types/IntegerType';
 import { StringType } from './types/StringType';
-import { SymbolTypeFlag } from './SymbolTableFlag';
+import { SymbolTypeFlag } from './SymbolTypeFlag';
 import { BooleanType } from './types/BooleanType';
 import { DoubleType } from './types/DoubleType';
 import { DynamicType } from './types/DynamicType';