Better member annotation scoping #4648
Merged
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
This also deems all annotations on non-whitespace lines illegal. The reasoning behind this change is that the previous system was somewhat unintuitive and fragile in that comments and non-member annotations could break a member annotation section. This is inconvenient in particular when removing annotations with a comment on the same line. Moreover, the new scoping is better aligned with VBE's own procedure separators. As a side-effect, the logic for determining the annotated line has been centralized and simplified. In addition, stacking identifier annotations along a procedure is no longer possible, which could be problematic before.
This switches getting the first endOfLine to a new dedicated extension that takes as much shortcuts as possible.
Previously, it first got all matches by walking the entire subtree, then ordered the results by start index and finally took the first one. Now, it uses a depth-first search approach to find the first result and bails out as soon as a match is found.
retailcoder
reviewed
Dec 19, 2018
| annotations.AddRange(annotationsStartingOnCurrentLine); | ||
| } | ||
| return annotations; | ||
| return _declarationFinder.FindAnnotations(module, line).Where(annotation => annotation.AnnotationType.HasFlag(AnnotationType.IdentifierAnnotation)); |
There was a problem hiding this comment.
Nice! Should this method be inlined at this point?
There was a problem hiding this comment.
That would make sense. Then, the entire class is needed no longer.
The only method does no longer contain a lot of logic; it is just one call to the `DeclarationFinder` followed by a single `Where` on the annotation type.
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.
This PR simplifies the annotation scoping.
After this PR, the rules will be as follows, where annotations and comments are considered to be whitespace.
AnnotationType.In addition, this PR changes the
GetDescendent<TContext>()extension to a depth first search approach from traversing the entire tree collecting all candidates, then sorting them and finally taking the first.