Fix: Projects skipped when missing EndProject #5808
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
When reading the solution .sln file each project is parsed by catching
the "Project(" sequence, and the parsing is stopped when reaching the
EndProject label.
However, in case of having the solution file malformed
(see: )
it is possible, that one of projects won't have it's EndProject label,
so originally it will be the only one being added to the projects list.
To avoid such situation, new condition has been added to the project's
parsing procedure, which responsibility is to check against additional
"Project(" sequence BEFORE meeting EndProject.
This situation indicates malformed solution file.
To handle this situation, additional 'else if' statement logs the
warning and recursively starts to parse another project.
When getting back from reading incorrectly nested project, the original
project's parsing procedure is stopped and whole procedure continues.
Two unit tests are added to cover the case with missing EndProject in an invalid solution file: ParseNextProjectContainedInInvalidSolutionEvenWhenMissingEndProject - which is to check for a case with one project after missing EndProject, ParseAllProjectsContainedInInvalidSolutionEvenWhenMissingEndProject - which is to check for a case with more than just one project after missing EndProject Both these tests should simply check whether each project, potentially skipped due to missing EndProject label, is correctly found in the solution file and is parsed correctly by recursive approach.
The unit test checking whether incorrectly nested project in malformed solution file (when first project missing it's EndProject label) is correctly found and parsed is redundant. The other unit test checking similar case (several projects nested under the one missing EndProject) is the superset of the first one, which makes the first one unnecessary.
|
Failing test looks like the problematic one improved by #5827. Reran it. |
|
Thanks, @BartoszKlonowski! |
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 pull request fixes: #5027
Changes introduced in this PR covers the case where having a project without EndProject label, next projects contained in this malformed solution .sln file are not built.
So this PR adjusts the behavior of MSBuild to the Visual Studio (please see #5027 (comment))
Changes done in this delivery are:
One for only one incorrectly nested project
One for multiple projects missing EndProject
NOTE: Recursive approach was necessary, due to impossibility of going one line back in the stream being already read, which means that when new
"Project("is spotted instead of"EndProject", it's already too late for the normal procedure to notice the next"Project(". There is aPeek()method available, which allows to see the next single character without actually reading it, but this approach would require bigger refactor, while recursive approach is now tested and verified working well.