-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Encode TargetPaths before writing them to the editorconfig #16858
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.SourceGenerators.targets
Outdated
Show resolved
Hide resolved
f21bd71
to
add5f17
Compare
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; just some clarifying questions for my understanding.
':' or '\\' or '/' => '_', | ||
var @default => @default, | ||
}); | ||
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 this still required when we have the below !char.IsLetterOrDigit
?
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.
Not really -- I mostly kept it so that it was clear treating path separators differently.
default: | ||
builder.Append(filePath[i]); | ||
break; | ||
} | ||
} | ||
|
||
return builder.ToString(); |
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.
So this is essentially used as a cache key?
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.
Yeah, this gets used as a the hint name for the generated SourceText. It's the filename that gets written to disk when the /p:EmitCompilerGeneratedFiles=true
is set true and is used as the identifier for the generated file.
Address dotnet/aspnetcore#28794.
Source generators and analyzers use an editor config file to transfer relevant metadata between MSBuild and the compiler. In Razor, the editor config is populated with the
TargetPaths
of the output files like so:If there are characters in the editor config that are delineated as special characters by the editor config spec, then the string processing will be borked for the filename. For example, a "#" anywhere in the filename will cause the rest of the filename to be ignored since # is interpreted as a comment character in editorconfig.
To work around this, we base64 encode the TargetPath property before writing it to disk to ensure that only alphanumeric characters are written to the editor config.
Then we decode the properties when reading them.
Note: this is a temporary workaround until a more permanent solution is in place at the compiler level.