rokucommunity / brighterscript

A superset of Roku's BrightScript language
MIT License
158 stars 46 forks source link

Validate single scope #1123

Closed markwpearce closed 3 months ago

markwpearce commented 5 months ago

With version 1, we will be changing how validation works. The goal is that all files will still be totally validated, except the method will be different. Currently, in v0.65, when a file changes, all files in all affected scopes are validated. This validation is rather basic - checking number of function arguments, verifying no duplicate function names, etc.

In version 1, the validation is much more rigorous, as all known types, namespaces, variables, etc. are fully checked for how they are used (as function args, return values, property access, etc.). This is taking MUCH longer to do, especially for files that are included in many scopes, like a utils file with common functions that’s included in every component. So we are going to experiment with only checking the first scope a file is in, then also producing validation errors if we expect there might be errors in other scopes.

For example, let’s say FileA references function someFunc that is not defined in the same file, and FileA is included in 2 different components.

If FileA changes, it will only be fully validated in the scope of the first component. In that context, if someFunc is defined (and its usage is consistent with its definition) there will be no error. However, we also check that someFunc is available (and compatible) in the context of the second scope.

If someFunc is not defined in the second component, or it has a different function signature, we will add a diagnostic error to say that the symbol is inconsistent across scopes.

Because we only need to fully check the files in the first scope, things go much quicker. The only downside is that the errors will be a little non-specific, eg. “Function someFunc is not compatible in Component A and Component B”, vs. if we fully validated, we know stuff like the properties of the return type of the function are different, and these are the differences, and it’s ok, or maybe a problem in the the context of the usage, etc.

markwpearce commented 5 months ago

Jellyfin_Roku validation:

Before:

Validate all scopes
Validate all scopes finished. (3s668.876ms)
Validation Metrics: filesValidated=307, fileValidationTime=236.509ms, fileInfoGenerationTime=91.165ms, programValidationTime=0.279ms, scopesValidated=138, totalLinkTime=681.913ms, totalScopeValidationTime=2s903.497ms, componentValidationTime=0.560ms
Validating project finished. (4s39.916ms)

After:

Validate all scopes
Validate all scopes finished. (1s447.582ms)
Validation Metrics: filesValidated=307, fileValidationTime=240.485ms, fileInfoGenerationTime=77.658ms, crossScopeValidationTime=78.467ms, scopesValidated=138, totalLinkTime=734.44ms, totalScopeValidationTime=637.439ms, componentValidationTime=0.558ms
Validating project finished. (1s885.752ms)
markwpearce commented 5 months ago

markwpearce commented 4 months ago

Ready for review!

markwpearce commented 3 months ago

Aliases are properly revalidated when source changes

https://github.com/rokucommunity/brighterscript/assets/810290/4e8a9540-c826-4b02-b39b-c34f2ac3c2ca