Skip to content

Commit

Permalink
Support relative paths in interceptors (#71879)
Browse files Browse the repository at this point in the history
  • Loading branch information
RikkiGibson authored Feb 27, 2024
1 parent 3e56610 commit 1fae4e6
Show file tree
Hide file tree
Showing 16 changed files with 742 additions and 112 deletions.
17 changes: 6 additions & 11 deletions docs/features/interceptors.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,15 @@ File-local declarations of this type (`file class InterceptsLocationAttribute`)

#### File paths

File paths used in `[InterceptsLocation]` are expected to have `/pathmap` substitution already applied. Generators should accomplish this by locally recreating the file path transformation performed by the compiler:
The *referenced syntax tree* of an `[InterceptsLocation]` is determined by normalizing the `filePath` argument value relative to the path of the containing syntax tree of the `[InterceptsLocation]` usage, similar to how paths in `#line` directives are normalized. Let this normalized path be called `normalizedInterceptorPath`. If exactly one syntax tree in the compilation has a normalized path which matches `normalizedInterceptorPath` by ordinal string comparison, that is the *referenced syntax tree*. Otherwise, an error occurs.

```cs
using Microsoft.CodeAnalysis;

string GetInterceptorFilePath(SyntaxTree tree, Compilation compilation)
{
return compilation.Options.SourceReferenceResolver?.NormalizePath(tree.FilePath, baseFilePath: null) ?? tree.FilePath;
}
```
`#line` directives are not considered when determining the call referenced by an `[InterceptsLocation]` attribute. In other words, the file path, line and column numbers used in `[InterceptsLocation]` are expected to refer to *unmapped* source locations.

The file path given in the attribute must be equal by ordinal comparison to the value given by the above function.
Temporarily, for compatibility purposes, when the initial matching strategy outlined above fails to match any syntax trees, we will fall back to a "compat" matching strategy which works in the following way:
- A *mapped path* of each syntax tree is determined by applying [`/pathmap`](https://learn.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.commandlinearguments.pathmap?view=roslyn-dotnet-4.7.0) substitution to `SyntaxTree.FilePath`.
- For a given `[InterceptsLocation]` usage, the `filePath` argument value is compared to the *mapped path* of each syntax tree using ordinal string comparison. If exactly one syntax tree matches under this comparison, that is the *referenced syntax tree*. Otherwise, an error occurs.

The compiler does not map `#line` directives when determining if an `[InterceptsLocation]` attribute intercepts a particular call in syntax.
Support for the "compat" strategy will be dropped prior to stable release. Tracked by https://github.com/dotnet/roslyn/issues/72265.

#### Position

Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/CommandLine/CSharpCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,9 @@ protected override void ResolveEmbeddedFilesFromExternalSourceDirectives(
}
}

private protected override GeneratorDriver CreateGeneratorDriver(ParseOptions parseOptions, ImmutableArray<ISourceGenerator> generators, AnalyzerConfigOptionsProvider analyzerConfigOptionsProvider, ImmutableArray<AdditionalText> additionalTexts)
private protected override GeneratorDriver CreateGeneratorDriver(string baseDirectory, ParseOptions parseOptions, ImmutableArray<ISourceGenerator> generators, AnalyzerConfigOptionsProvider analyzerConfigOptionsProvider, ImmutableArray<AdditionalText> additionalTexts)
{
return CSharpGeneratorDriver.Create(generators, additionalTexts, (CSharpParseOptions)parseOptions, analyzerConfigOptionsProvider);
return CSharpGeneratorDriver.Create(generators, additionalTexts, (CSharpParseOptions)parseOptions, analyzerConfigOptionsProvider, driverOptions: new GeneratorDriverOptions() { BaseDirectory = baseDirectory });
}

private protected override void DiagnoseBadAccesses(TextWriter consoleOutput, ErrorLogger? errorLogger, Compilation compilation, ImmutableArray<Diagnostic> diagnostics)
Expand Down
35 changes: 34 additions & 1 deletion src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,13 @@ internal Conversions Conversions
/// </summary>
private readonly ConcurrentCache<TypeSymbol, NamedTypeSymbol> _typeToNullableVersion = new ConcurrentCache<TypeSymbol, NamedTypeSymbol>(size: 100);

/// <summary>Lazily caches SyntaxTrees by their mapped path. Used to look up the syntax tree referenced by an interceptor. <see cref="TryGetInterceptor"/></summary>
/// <summary>Lazily caches SyntaxTrees by their mapped path. Used to look up the syntax tree referenced by an interceptor (temporary compat behavior).</summary>
/// <remarks>Must be removed prior to interceptors stable release.</remarks>
private ImmutableSegmentedDictionary<string, OneOrMany<SyntaxTree>> _mappedPathToSyntaxTree;

/// <summary>Lazily caches SyntaxTrees by their path. Used to look up the syntax tree referenced by an interceptor.</summary>
private ImmutableSegmentedDictionary<string, OneOrMany<SyntaxTree>> _pathToSyntaxTree;

public override string Language
{
get
Expand Down Expand Up @@ -1042,6 +1046,9 @@ internal override int GetSyntaxTreeOrdinal(SyntaxTree tree)

internal OneOrMany<SyntaxTree> GetSyntaxTreesByMappedPath(string mappedPath)
{
// This method supports a "compat" behavior for interceptor file path resolution.
// It must be removed prior to stable release.

// We could consider storing this on SyntaxAndDeclarationManager instead, and updating it incrementally.
// However, this would make it more difficult for it to be "pay-for-play",
// i.e. only created in compilations where interceptors are used.
Expand All @@ -1067,6 +1074,32 @@ ImmutableSegmentedDictionary<string, OneOrMany<SyntaxTree>> computeMappedPathToS
}
}

internal OneOrMany<SyntaxTree> GetSyntaxTreesByPath(string path)
{
// We could consider storing this on SyntaxAndDeclarationManager instead, and updating it incrementally.
// However, this would make it more difficult for it to be "pay-for-play",
// i.e. only created in compilations where interceptors are used.
var pathToSyntaxTree = _pathToSyntaxTree;
if (pathToSyntaxTree.IsDefault)
{
RoslynImmutableInterlocked.InterlockedInitialize(ref _pathToSyntaxTree, computePathToSyntaxTree());
pathToSyntaxTree = _pathToSyntaxTree;
}

return pathToSyntaxTree.TryGetValue(path, out var value) ? value : OneOrMany<SyntaxTree>.Empty;

ImmutableSegmentedDictionary<string, OneOrMany<SyntaxTree>> computePathToSyntaxTree()
{
var builder = ImmutableSegmentedDictionary.CreateBuilder<string, OneOrMany<SyntaxTree>>();
foreach (var tree in SyntaxTrees)
{
var path = FileUtilities.GetNormalizedPathOrOriginalPath(tree.FilePath, basePath: null);
builder[path] = builder.ContainsKey(path) ? builder[path].Add(tree) : OneOrMany.Create(tree);
}
return builder.ToImmutable();
}
}

#endregion

#region References
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2202,7 +2202,7 @@ internal enum ErrorCode
ERR_InterceptorLineOutOfRange = 9142,
ERR_InterceptorCharacterOutOfRange = 9143,
ERR_InterceptorSignatureMismatch = 9144,
ERR_InterceptorPathNotInCompilationWithUnmappedCandidate = 9145,
// ERR_InterceptorPathNotInCompilationWithUnmappedCandidate = 9145,
ERR_InterceptorMethodMustBeOrdinary = 9146,
ERR_InterceptorMustReferToStartOfTokenPosition = 9147,
ERR_InterceptorMustHaveMatchingThisParameter = 9148,
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2349,7 +2349,6 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_InterceptorContainingTypeCannotBeGeneric:
case ErrorCode.ERR_InterceptorPathNotInCompilation:
case ErrorCode.ERR_InterceptorPathNotInCompilationWithCandidate:
case ErrorCode.ERR_InterceptorPathNotInCompilationWithUnmappedCandidate:
case ErrorCode.ERR_InterceptorPositionBadToken:
case ErrorCode.ERR_InterceptorLineOutOfRange:
case ErrorCode.ERR_InterceptorCharacterOutOfRange:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1000,54 +1000,56 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments
return;
}

var syntaxTrees = DeclaringCompilation.SyntaxTrees;
var matchingTrees = DeclaringCompilation.GetSyntaxTreesByMappedPath(attributeFilePath);
var normalizedPath = FileUtilities.GetNormalizedPathOrOriginalPath(attributeFilePath, basePath: SyntaxTree.FilePath);
var matchingTrees = DeclaringCompilation.GetSyntaxTreesByPath(normalizedPath);
if (matchingTrees.Count > 1)
{
diagnostics.Add(ErrorCode.ERR_InterceptorNonUniquePath, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), normalizedPath);
return;
}

if (matchingTrees.Count == 0)
{
var referenceResolver = DeclaringCompilation.Options.SourceReferenceResolver;
// if we expect '/_/Program.cs':
// Temporary compat behavior: check if 'attributeFilePath' is equivalent to a mapped path of one of the syntax trees in the compilation.
matchingTrees = DeclaringCompilation.GetSyntaxTreesByMappedPath(attributeFilePath);

// we might get: 'C:\Project\Program.cs' <-- path not mapped
var unmappedMatch = syntaxTrees.FirstOrDefault(static (tree, filePath) => tree.FilePath == filePath, attributeFilePath);
if (unmappedMatch != null)
if (matchingTrees.Count > 1)
{
diagnostics.Add(
ErrorCode.ERR_InterceptorPathNotInCompilationWithUnmappedCandidate,
attributeData.GetAttributeArgumentLocation(filePathParameterIndex),
attributeFilePath,
mapPath(referenceResolver, unmappedMatch));
diagnostics.Add(ErrorCode.ERR_InterceptorNonUniquePath, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), attributeFilePath);
return;
}
}

// we might get: '\_\Program.cs' <-- slashes not normalized
// we might get: '\_/Program.cs' <-- slashes don't match
// Neither the primary or compat methods of resolving the file path found any match.
if (matchingTrees.Count == 0)
{
// if we expect '/src/Program.cs':
// we might get: 'Program.cs' <-- suffix match
// Force normalization of all '\' to '/', but when we recommend a path in the diagnostic message, ensure it will match what we expect if the user decides to use it.
var suffixMatch = syntaxTrees.FirstOrDefault(static (tree, pair)
=> mapPath(pair.referenceResolver, tree)
var syntaxTrees = DeclaringCompilation.SyntaxTrees;
var suffixMatch = syntaxTrees.FirstOrDefault(static (tree, attributeFilePathWithForwardSlashes)
=> tree.FilePath
.Replace('\\', '/')
.EndsWith(pair.attributeFilePath),
(referenceResolver, attributeFilePath: attributeFilePath.Replace('\\', '/')));
.EndsWith(attributeFilePathWithForwardSlashes),
attributeFilePath.Replace('\\', '/'));
if (suffixMatch != null)
{
var recommendedPath = PathUtilities.IsAbsolute(SyntaxTree.FilePath)
? PathUtilities.GetRelativePath(PathUtilities.GetDirectoryName(SyntaxTree.FilePath), suffixMatch.FilePath)
: suffixMatch.FilePath;
diagnostics.Add(
ErrorCode.ERR_InterceptorPathNotInCompilationWithCandidate,
attributeData.GetAttributeArgumentLocation(filePathParameterIndex),
attributeFilePath,
mapPath(referenceResolver, suffixMatch));
recommendedPath);
return;
}

diagnostics.Add(ErrorCode.ERR_InterceptorPathNotInCompilation, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), attributeFilePath);
diagnostics.Add(ErrorCode.ERR_InterceptorPathNotInCompilation, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), normalizedPath);

return;
}
else if (matchingTrees.Count > 1)
{
diagnostics.Add(ErrorCode.ERR_InterceptorNonUniquePath, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), attributeFilePath);
return;
}

Debug.Assert(matchingTrees.Count == 1);
SyntaxTree? matchingTree = matchingTrees[0];
// Internally, line and character numbers are 0-indexed, but when they appear in code or diagnostic messages, they are 1-indexed.
int lineNumberZeroBased = lineNumberOneBased - 1;
Expand Down Expand Up @@ -1107,11 +1109,6 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments

DeclaringCompilation.AddInterception(matchingTree.FilePath, lineNumberZeroBased, characterNumberZeroBased, attributeLocation, this);

static string mapPath(SourceReferenceResolver? referenceResolver, SyntaxTree tree)
{
return referenceResolver?.NormalizePath(tree.FilePath, baseFilePath: null) ?? tree.FilePath;
}

// Caller must free the returned builder.
ArrayBuilder<string> getNamespaceNames()
{
Expand Down
Loading

0 comments on commit 1fae4e6

Please sign in to comment.