Skip to content
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

Provide MSB5009 error with project's name and it's GUID #5835

Conversation

BartoszKlonowski
Copy link
Contributor

This pull request fixes: #4836

Changes introduced in this delivery extends the MSB5009 with additional information about the incorrectly nested project within the solution. Additional details are:

  • incorrectly nested project's name (without the full path, unique name only),
  • incorrectly nested project's GUID

NOTE: original MSB5009 error is kept in the resources for backward compatibility (see commit messages) while the extended MSB5009 can be easily invoked with extended name.


Changes done in this delivery are:

  • Create additional version of MSB5009 error with modified error message - meaning stays the same as original MSB5009 error, but incorrectly nested project's name and GUID is added as arguments.
  • Replace the old MSB5009 with the extended one for each place where this extended error should be used.
    NOTE: SolutionFile.ParseNestedProjects() method has the original error kept, as it does specify at least the line of the solution file with the incorrect project. Moreover, at the moment of throwing the error, there's no possibility of getting the name of the project.
  • Cover the incorrect nesting case with the unit test checking whether for an original reproduction solution (please check the commit message for link) proper error with both name and GUID is thrown.

For more details about each change please see the commit message.


Example
of original MSB5009 error vs the extended version presented below. For both cases the same solution file was used, which is the solution file mentioned before available to get from the original issue.

The original error:
obraz
The extended version (the results of this following PR):
obraz

BartoszKlonowski and others added 4 commits October 23, 2020 12:46
The MSB5009 - SolutionParseNestedProjectError has been printed for a
certain project but without specifying the project's name or even GUID
which was making this issue difficult to debug.
To make it more developer/user friendly, the MSB5009 error has been
provided with the invalid project's name and GUID embedded in the error
message.
Message is extended by string arguments separate for ProjectName and
ProjectGUID, so from now on it's required to add them as a
BuildEventFileInfo constructor arguments.

NOTE: Though error message is automaticaly generated for each language,
some are still not translated.
The extended MSB5009 error message requires ProjectName and ProjectGuid
as arguments otherwise raw "{0}" will be displayed to the console in
case of error.
The MSB5009 error messsage is used in other places than the one covered
in the extension implementation, so each place using this error has to
be covered with the changes.

NOTE: The only exception is the SolutionFile.ParseNestedProjects() method
which uses the MSB5009 error too, but at the point of calling this error,
there are no project's name nor GUID available to display, so it should
continue to use the original error message string.
To make it possible:
 - the original error string has been kept,
 - while the extended one has been renamed to
   "SolutionParseNestedProjectErrorWithNameAndGuid"
   to indicate it's extended content and required arguments

Leaving the MSB5009 error message in the original form for
SolutionFile.ParseNestedProject() method doesn't mean that no
information will be given to developer/user.
Note that it already calls the BuildEventFileInfo constructor with full
path and current line's number, which will ease the debugging in case of
having MSB5009 error.
New unit test covering the enhanced MSB5009 error checks whether:
 - MSB5009 error is thrown for incorrectly nested project in solution
 - error contains the name of incorrectly nested project
 - error contains the GUID of incorrectly nested project

NOTE: The solution file used for this unit test is the original solution
file given as an example of the original error which these changes fix.
Please see: dotnet#4835
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Your test did what the other tests around here did, but Shouldly offers us a nicer way to do it, so I pushed a switch to that mechanism.

Including @KathleenDollard for opinions on the error message (in addition to mine).

src/Shared/Resources/Strings.shared.resx Outdated Show resolved Hide resolved
Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Pending string review.

src/Build.UnitTests/Construction/SolutionFile_Tests.cs Outdated Show resolved Hide resolved
Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI errors were just because the translations were out-of-date. You can fix that by building.

src/Build/Construction/Solution/ProjectInSolution.cs Outdated Show resolved Hide resolved
src/Build/Construction/Solution/ProjectInSolution.cs Outdated Show resolved Hide resolved
@BartoszKlonowski
Copy link
Contributor Author

I was just about to deliver the review remarks, but thank you for the commitment!

@rainersigwald rainersigwald merged commit daead5f into dotnet:master Nov 20, 2020
@rainersigwald
Copy link
Member

Thanks, @BartoszKlonowski

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider Enhancing MSB5009 To Indicate Which Project Was Incorrect
4 participants