-
Notifications
You must be signed in to change notification settings - Fork 1.2k
VirtualProjectBuilder surface cleanup #53088
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,15 +36,15 @@ | |
| /// The latter is useful for <c>dotnet run file.cs</c> where if there are app directives after the first token, | ||
| /// compiler reports <see cref="ErrorCode.ERR_PPIgnoredFollowsToken"/> anyway, so we speed up success scenarios by not parsing the whole file up front in the SDK CLI. | ||
| /// </param> | ||
| public static ImmutableArray<CSharpDirective> FindDirectives(SourceFile sourceFile, bool reportAllErrors, ErrorReporter reportError) | ||
| public static ImmutableArray<CSharpDirective> FindDirectives(SourceFile sourceFile, bool reportAllErrors, ErrorReporter errorReporter) | ||
|
Check failure on line 39 in src/Cli/Microsoft.DotNet.FileBasedPrograms/FileLevelDirectiveHelpers.cs
|
||
| { | ||
| var builder = ImmutableArray.CreateBuilder<CSharpDirective>(); | ||
| var tokenizer = CreateTokenizer(sourceFile.Text); | ||
|
|
||
| var result = tokenizer.ParseLeadingTrivia(); | ||
| var triviaList = result.Token.LeadingTrivia; | ||
|
|
||
| FindLeadingDirectives(sourceFile, triviaList, reportError, builder); | ||
| FindLeadingDirectives(sourceFile, triviaList, errorReporter, builder); | ||
|
|
||
| // In conversion mode, we want to report errors for any invalid directives in the rest of the file | ||
| // so users don't end up with invalid directives in the converted project. | ||
|
|
@@ -73,7 +73,7 @@ | |
| { | ||
| if (trivia.ContainsDiagnostics && trivia.IsKind(SyntaxKind.IgnoredDirectiveTrivia)) | ||
| { | ||
| reportError(sourceFile, trivia.Span, FileBasedProgramsResources.CannotConvertDirective); | ||
| errorReporter(sourceFile.Text, sourceFile.Path, trivia.Span, FileBasedProgramsResources.CannotConvertDirective); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -83,10 +83,10 @@ | |
|
|
||
| /// <summary>Finds file-level directives in the leading trivia list of a compilation unit and reports diagnostics on them.</summary> | ||
| /// <param name="builder">The builder to store the parsed directives in, or null if the parsed directives are not needed.</param> | ||
| public static void FindLeadingDirectives( | ||
|
Check failure on line 86 in src/Cli/Microsoft.DotNet.FileBasedPrograms/FileLevelDirectiveHelpers.cs
|
||
| SourceFile sourceFile, | ||
| SyntaxTriviaList triviaList, | ||
| ErrorReporter reportError, | ||
| ErrorReporter errorReporter, | ||
| ImmutableArray<CSharpDirective>.Builder? builder) | ||
| { | ||
| Debug.Assert(triviaList.Span.Start == 0); | ||
|
|
@@ -144,7 +144,7 @@ | |
| LeadingWhiteSpace = whiteSpace.Leading, | ||
| TrailingWhiteSpace = whiteSpace.Trailing, | ||
| }, | ||
| ReportError = reportError, | ||
| ErrorReporter = errorReporter, | ||
| SourceFile = sourceFile, | ||
| DirectiveKind = name, | ||
| DirectiveText = value, | ||
|
|
@@ -153,7 +153,7 @@ | |
| // Block quotes now so we can later support quoted values without a breaking change. https://github.com/dotnet/sdk/issues/49367 | ||
| if (value.Contains('"')) | ||
| { | ||
| reportError(sourceFile, context.Info.Span, FileBasedProgramsResources.QuoteInDirective); | ||
| context.ReportError(FileBasedProgramsResources.QuoteInDirective); | ||
| } | ||
|
|
||
| if (CSharpDirective.Parse(context) is { } directive) | ||
|
|
@@ -162,7 +162,7 @@ | |
| if (deduplicated.TryGetValue(directive, out var existingDirective)) | ||
| { | ||
| var typeAndName = $"#:{existingDirective.GetType().Name.ToLowerInvariant()} {existingDirective.Name}"; | ||
| reportError(sourceFile, directive.Info.Span, string.Format(FileBasedProgramsResources.DuplicateDirective, typeAndName)); | ||
| context.ReportError(directive.Info.Span, string.Format(FileBasedProgramsResources.DuplicateDirective, typeAndName)); | ||
| } | ||
| else | ||
| { | ||
|
|
@@ -231,11 +231,6 @@ | |
| return new SourceFile(filePath, SourceText.From(stream, encoding: null)); | ||
| } | ||
|
|
||
| public SourceFile WithText(SourceText newText) | ||
| { | ||
| return new SourceFile(Path, newText); | ||
| } | ||
|
|
||
| public void Save() | ||
| { | ||
| using var stream = File.Open(Path, FileMode.Create, FileAccess.Write); | ||
|
|
@@ -244,17 +239,6 @@ | |
| using var writer = new StreamWriter(stream, encoding); | ||
| Text.Write(writer); | ||
| } | ||
|
|
||
| public FileLinePositionSpan GetFileLinePositionSpan(TextSpan span) | ||
| { | ||
| return new FileLinePositionSpan(Path, Text.Lines.GetLinePositionSpan(span)); | ||
| } | ||
|
|
||
| public string GetLocationString(TextSpan span) | ||
| { | ||
| var positionSpan = GetFileLinePositionSpan(span); | ||
| return $"{positionSpan.Path}({positionSpan.StartLinePosition.Line + 1})"; | ||
| } | ||
| } | ||
|
|
||
| internal static partial class Patterns | ||
|
|
@@ -293,10 +277,16 @@ | |
| public readonly struct ParseContext | ||
| { | ||
| public required ParseInfo Info { get; init; } | ||
| public required ErrorReporter ReportError { get; init; } | ||
| public required ErrorReporter ErrorReporter { get; init; } | ||
|
Check failure on line 280 in src/Cli/Microsoft.DotNet.FileBasedPrograms/FileLevelDirectiveHelpers.cs
|
||
| public required SourceFile SourceFile { get; init; } | ||
| public required string DirectiveKind { get; init; } | ||
| public required string DirectiveText { get; init; } | ||
|
|
||
| public void ReportError(string message) | ||
|
Check failure on line 285 in src/Cli/Microsoft.DotNet.FileBasedPrograms/FileLevelDirectiveHelpers.cs
|
||
| => ErrorReporter(SourceFile.Text, SourceFile.Path, Info.Span, message); | ||
|
|
||
| public void ReportError(TextSpan span, string message) | ||
|
Check failure on line 288 in src/Cli/Microsoft.DotNet.FileBasedPrograms/FileLevelDirectiveHelpers.cs
|
||
| => ErrorReporter(SourceFile.Text, SourceFile.Path, span, message); | ||
| } | ||
|
|
||
| public static Named? Parse(in ParseContext context) | ||
|
|
@@ -308,7 +298,7 @@ | |
| case "package": return Package.Parse(context); | ||
| case "project": return Project.Parse(context); | ||
| default: | ||
| context.ReportError(context.SourceFile, context.Info.Span, string.Format(FileBasedProgramsResources.UnrecognizedDirective, context.DirectiveKind)); | ||
| context.ReportError(string.Format(FileBasedProgramsResources.UnrecognizedDirective, context.DirectiveKind)); | ||
| return null; | ||
| }; | ||
| } | ||
|
|
@@ -321,14 +311,14 @@ | |
| string directiveKind = context.DirectiveKind; | ||
| if (firstPart.IsWhiteSpace()) | ||
| { | ||
| context.ReportError(context.SourceFile, context.Info.Span, string.Format(FileBasedProgramsResources.MissingDirectiveName, directiveKind)); | ||
| context.ReportError(string.Format(FileBasedProgramsResources.MissingDirectiveName, directiveKind)); | ||
| return null; | ||
| } | ||
|
|
||
| // If the name contains characters that resemble separators, report an error to avoid any confusion. | ||
| if (Patterns.DisallowedNameCharacters.Match(context.DirectiveText, beginning: 0, length: firstPart.Length).Success) | ||
| { | ||
| context.ReportError(context.SourceFile, context.Info.Span, string.Format(FileBasedProgramsResources.InvalidDirectiveName, directiveKind, separator)); | ||
| context.ReportError(string.Format(FileBasedProgramsResources.InvalidDirectiveName, directiveKind, separator)); | ||
| return null; | ||
| } | ||
|
|
||
|
|
@@ -404,7 +394,7 @@ | |
|
|
||
| if (propertyValue is null) | ||
| { | ||
| context.ReportError(context.SourceFile, context.Info.Span, FileBasedProgramsResources.PropertyDirectiveMissingParts); | ||
| context.ReportError(FileBasedProgramsResources.PropertyDirectiveMissingParts); | ||
| return null; | ||
| } | ||
|
|
||
|
|
@@ -414,14 +404,14 @@ | |
| } | ||
| catch (XmlException ex) | ||
| { | ||
| context.ReportError(context.SourceFile, context.Info.Span, string.Format(FileBasedProgramsResources.PropertyDirectiveInvalidName, ex.Message)); | ||
| context.ReportError(string.Format(FileBasedProgramsResources.PropertyDirectiveInvalidName, ex.Message)); | ||
| return null; | ||
| } | ||
|
|
||
| if (propertyName.Equals("RestoreUseStaticGraphEvaluation", StringComparison.OrdinalIgnoreCase) && | ||
| MSBuildUtilities.ConvertStringToBool(propertyValue)) | ||
| { | ||
| context.ReportError(context.SourceFile, context.Info.Span, FileBasedProgramsResources.StaticGraphRestoreNotSupported); | ||
| context.ReportError(FileBasedProgramsResources.StaticGraphRestoreNotSupported); | ||
| } | ||
|
|
||
| return new Property(context.Info) | ||
|
|
@@ -493,8 +483,7 @@ | |
| var directiveText = context.DirectiveText; | ||
| if (directiveText.IsWhiteSpace()) | ||
| { | ||
| string directiveKind = context.DirectiveKind; | ||
| context.ReportError(context.SourceFile, context.Info.Span, string.Format(FileBasedProgramsResources.MissingDirectiveName, directiveKind)); | ||
| context.ReportError(string.Format(FileBasedProgramsResources.MissingDirectiveName, context.DirectiveKind)); | ||
| return null; | ||
| } | ||
|
|
||
|
|
@@ -532,14 +521,15 @@ | |
| /// <summary> | ||
| /// If the directive points to a directory, returns a new directive pointing to the corresponding project file. | ||
| /// </summary> | ||
| public Project EnsureProjectFilePath(SourceFile sourceFile, ErrorReporter reportError) | ||
| public Project EnsureProjectFilePath(SourceFile sourceFile, ErrorReporter errorReporter) | ||
|
Check failure on line 524 in src/Cli/Microsoft.DotNet.FileBasedPrograms/FileLevelDirectiveHelpers.cs
|
||
| { | ||
| var resolvedName = Name; | ||
| var sourcePath = sourceFile.Path; | ||
|
|
||
| // If the path is a directory like '../lib', transform it to a project file path like '../lib/lib.csproj'. | ||
| // Also normalize backslashes to forward slashes to ensure the directive works on all platforms. | ||
| var sourceDirectory = Path.GetDirectoryName(sourceFile.Path) | ||
| ?? throw new InvalidOperationException($"Source file path '{sourceFile.Path}' does not have a containing directory."); | ||
| var sourceDirectory = Path.GetDirectoryName(sourcePath) | ||
| ?? throw new InvalidOperationException($"Source file path '{sourcePath}' does not have a containing directory."); | ||
|
|
||
| var resolvedProjectPath = Path.Combine(sourceDirectory, resolvedName.Replace('\\', '/')); | ||
| if (Directory.Exists(resolvedProjectPath)) | ||
|
|
@@ -553,16 +543,18 @@ | |
| } | ||
| else | ||
| { | ||
| reportError(sourceFile, Info.Span, string.Format(FileBasedProgramsResources.InvalidProjectDirective, error)); | ||
| ReportError(string.Format(FileBasedProgramsResources.InvalidProjectDirective, error)); | ||
| } | ||
| } | ||
| else if (!File.Exists(resolvedProjectPath)) | ||
| { | ||
| reportError(sourceFile, Info.Span, | ||
| string.Format(FileBasedProgramsResources.InvalidProjectDirective, string.Format(FileBasedProgramsResources.CouldNotFindProjectOrDirectory, resolvedProjectPath))); | ||
| ReportError(string.Format(FileBasedProgramsResources.InvalidProjectDirective, string.Format(FileBasedProgramsResources.CouldNotFindProjectOrDirectory, resolvedProjectPath))); | ||
| } | ||
|
|
||
| return WithName(resolvedName, NameKind.ProjectFilePath); | ||
|
|
||
| void ReportError(string message) | ||
| => errorReporter(sourceFile.Text, sourcePath, Info.Span, message); | ||
| } | ||
|
|
||
| public override string ToString() => $"#:project {Name}"; | ||
|
|
@@ -617,25 +609,25 @@ | |
| } | ||
| } | ||
|
|
||
| internal delegate void ErrorReporter(SourceFile sourceFile, TextSpan textSpan, string message); | ||
| internal delegate void ErrorReporter(SourceText text, string path, TextSpan textSpan, string message); | ||
|
Check failure on line 612 in src/Cli/Microsoft.DotNet.FileBasedPrograms/FileLevelDirectiveHelpers.cs
|
||
|
|
||
| internal static partial class ErrorReporters | ||
| { | ||
| public static readonly ErrorReporter IgnoringReporter = | ||
| static (_, _, _) => { }; | ||
| static (_, _, _, _) => { }; | ||
|
|
||
| public static ErrorReporter CreateCollectingReporter(out ImmutableArray<SimpleDiagnostic>.Builder builder) | ||
| { | ||
| var capturedBuilder = builder = ImmutableArray.CreateBuilder<SimpleDiagnostic>(); | ||
|
|
||
| return (sourceFile, textSpan, message) => | ||
| return (text, path, textSpan, message) => | ||
| capturedBuilder.Add(new SimpleDiagnostic | ||
| { | ||
| Location = new SimpleDiagnostic.Position() | ||
| { | ||
| Path = sourceFile.Path, | ||
| Path = path, | ||
| TextSpan = textSpan, | ||
| Span = sourceFile.GetFileLinePositionSpan(textSpan).Span | ||
| Span = text.Lines.GetLinePositionSpan(textSpan) | ||
| }, | ||
| Message = message | ||
| }); | ||
|
|
||
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.
We should probably decide on naming and consolidate it everywhere. I think I've been renaming some parameters in the other direction (to
reportError) as part of #52347.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 renamed to avoid collision between helpers Context.ReportError and ErrorReporter ReportError field.