-
Notifications
You must be signed in to change notification settings - Fork 223
Don't replace the slashes in hint names #12477
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
Conversation
| { | ||
| switch (filePath[i]) | ||
| { | ||
| case ':' or '\\' or '/': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
':' is covered by the case below anyway.
davidwengier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have thought tests in ComputedTargetPathTest.cs would fail with this, at least.
Either way, can you add this test. I haven't run it, obviously, but it fails without your change, and I think it should pass with it.
[Theory]
[InlineData(true, false)]
[InlineData(true, true)]
[InlineData(false, false)]
public async Task TwoDocumentsWithTheSameBaseHintName(bool projectPath, bool generateConfigFile)
{
var builder = new RazorProjectBuilder
{
ProjectFilePath = projectPath ? TestProjectData.SomeProject.FilePath : null,
GenerateGlobalConfigFile = generateConfigFile,
GenerateAdditionalDocumentMetadata = false,
GenerateMSBuildProjectDirectory = false
};
var doc1Id = builder.AddAdditionalDocument(FilePath(@"Pages\Index.razor"), SourceText.From(""));
var doc2Id = builder.AddAdditionalDocument(FilePath(@"Pages_Index.razor"), SourceText.From(""));
var solution = LocalWorkspace.CurrentSolution;
solution = builder.Build(solution);
var doc1 = solution.GetAdditionalDocument(doc1Id).AssumeNotNull();
var doc2 = solution.GetAdditionalDocument(doc2Id).AssumeNotNull();
var generatedDocument = await doc1.Project.TryGetSourceGeneratedDocumentForRazorDocumentAsync(doc1, DisposalToken);
Assert.NotNull(generatedDocument);
Assert.Equal($"{s_hintNamePrefix}_Pages\\Index_razor.g.cs", generatedDocument.HintName);
generatedDocument = await doc2.Project.TryGetSourceGeneratedDocumentForRazorDocumentAsync(doc2, DisposalToken);
Assert.NotNull(generatedDocument);
Assert.Equal($"{s_hintNamePrefix}_Pages_Index_razor.g.cs", generatedDocument.HintName);
}
|
@davidwengier Ah they do, I just didn't see them because I was only running the net8.0 tests. Will add the extra test. |
davidwengier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, on the assumption that the remaining test failures are just things in need of infra or baseline updates
Fixes #11578
When the generator was originally written slashes were not allowed in hint names. That restriction was subsequently lifted, and they are valid parts of a hint name now. See here.