-
Notifications
You must be signed in to change notification settings - Fork 47
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
Validate single scope #1123
Merged
Merged
Validate single scope #1123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jellyfin_Roku validation: Before:
After:
|
…ires vs provides checking
⚡ |
…cope validation, and modifies scopeName sorting
Co-authored-by: Mark Pearce <markwpearce@gmail.com>
Aliases are properly revalidated when source changes AliasRevalidation.mov |
…hterscript into validate_single_scope
…y/brighterscript into validate_single_scope
…hterscript into validate_single_scope
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 functionsomeFunc
that is not defined in the same file, andFileA
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, ifsomeFunc
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 inComponent A
andComponent 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.