Skip to content

Commit

Permalink
Add setting and behavior to skip checks if dependency isn't direct
Browse files Browse the repository at this point in the history 
Up to now, we were running the SL check even if the project did not have a direct dependency with SL or the sponsorable package. This is because analyzers are (by design, for now?) transitive and this can't even be stopped via NuGet (see dotnet/sdk#1212 and NuGet/Home#6279).

We now introduce a `transitive` setting (defaulting to false) that is combined with the package dependencies to detect the indirect-ness and if so, skip the check.

Note that unless all the targets are in place to surface this indirect-ness to the analyzer, the check *will* be performed. This shields against the scenario where users might tweak targets to try to avoid getting the check to run at all.
  • Loading branch information
@kzu
kzu committed Aug 10, 2023
1 parent 00434ef commit fdf607d
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 17 deletions.
4 changes: 1 addition & 3 deletions src/Package/PublicAPI.Shipped.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@ static Devlooped.DiagnosticExtensions.IsKind(this Microsoft.CodeAnalysis.Diagnos
Devlooped.SponsorLinkSettings.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
Devlooped.SponsorLinkSettings.SupportedDiagnostics.set -> void
virtual Devlooped.SponsorLink.OnDiagnostic(string! projectPath, Devlooped.DiagnosticKind kind) -> Microsoft.CodeAnalysis.Diagnostic?
static Devlooped.SponsorLinkSettings.Create(string! sponsorable, string! product) -> Devlooped.SponsorLinkSettings!
static Devlooped.SponsorLinkSettings.Create(string! sponsorable, string! product, string? packageId = null, string? diagnosticsIdPrefix = null, int pauseMin = 0, int pauseMax = 4000) -> Devlooped.SponsorLinkSettings!
static Devlooped.SponsorLinkSettings.Create(string! sponsorable, string! product, string? packageId = null, string? version = null, string? diagnosticsIdPrefix = null, int pauseMin = 0, int pauseMax = 4000, int? quietDays = null) -> Devlooped.SponsorLinkSettings!
static Devlooped.SponsorLinkSettings.Create(string! sponsorable, string! product) -> Devlooped.SponsorLinkSettings!
5 changes: 4 additions & 1 deletion src/Package/PublicAPI.Unshipped.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,7 @@ Devlooped.SponsorStatus
Devlooped.SponsorStatus.AppMissing = 0 -> Devlooped.SponsorStatus
Devlooped.SponsorStatus.NotSponsoring = 1 -> Devlooped.SponsorStatus
Devlooped.SponsorStatus.Sponsoring = 2 -> Devlooped.SponsorStatus
static Devlooped.SponsorCheck.CheckAsync(string! workingDirectory, string! sponsorable, string! product, string? packageId = null, string? version = null, System.Net.Http.HttpClient? http = null) -> System.Threading.Tasks.Task<Devlooped.SponsorStatus?>!
static Devlooped.SponsorCheck.CheckAsync(string! workingDirectory, string! sponsorable, string! product, string? packageId = null, string? version = null, System.Net.Http.HttpClient? http = null) -> System.Threading.Tasks.Task<Devlooped.SponsorStatus?>!
static Devlooped.SponsorLinkSettings.Create(string! sponsorable, string! product, string? packageId = null, string? version = null, string? diagnosticsIdPrefix = null, int pauseMin = 0, int pauseMax = 4000, int? quietDays = null, bool transitive = false) -> Devlooped.SponsorLinkSettings!
static Devlooped.SponsorLinkSettings.Create(string! sponsorable, string! product, string? packageId, string? diagnosticsIdPrefix, int pauseMin, int pauseMax) -> Devlooped.SponsorLinkSettings!
static Devlooped.SponsorLinkSettings.Create(string! sponsorable, string! product, string? packageId, string? version, string? diagnosticsIdPrefix, int pauseMin, int pauseMax, int? quietDays) -> Devlooped.SponsorLinkSettings!
39 changes: 32 additions & 7 deletions src/Package/SponsorLink.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,23 @@ void AnalyzeSponsors(CompilationAnalysisContext context)
return;
}

var dependency = context.Options.AdditionalFiles
.Where(x => context.Options.AnalyzerConfigOptionsProvider.GetOptions(x) is var fileOptions &&
fileOptions.TryGetValue("build_metadata.AdditionalFiles.SourceItemType", out var itemType) &&
itemType == "PackageDependencies" &&
fileOptions.TryGetValue("build_metadata.AdditionalFiles.SourceIdentity", out var itemSpec) &&
itemSpec == settings.PackageId)
.Select(x => context.Options.AnalyzerConfigOptionsProvider.GetOptions(x).TryGetValue("build_metadata.AdditionalFiles.ParentPackage", out var parent) ?
parent : null)
.ToImmutableList();

// We should not perform the check if the project is not referencing the package
// We detect this by checking the dependencies (see runtime targets which add
// PackageDependencies for the settings.PackageId from @SponsorablePackageId),
// and if the dependency is found and has a non-null ParentPackage, it means it's
// a transitive dependency.
// Note that we default to being non-transitive.
var shouldSkip = !settings.Transitive && dependency.Any(x => x != null);

var (insideEditor, designTimeBuild) = ReadOptions(globalOptions);
var info = new BuildInfo(projectFile!, insideEditor, designTimeBuild);
Expand Down Expand Up @@ -500,15 +517,23 @@ void ReportExisting(CompilationAnalysisContext context, string? projectFile)

static Diagnostic WriteMessage(string sponsorable, string product, string projectDir, Diagnostic diag)
{
var objDir = Path.Combine(projectDir, "obj", "SponsorLink", sponsorable, product);
if (Directory.Exists(objDir))
try
{
foreach (var file in Directory.EnumerateFiles(objDir))
File.Delete(file);
}
var objDir = Path.Combine(projectDir, "obj", "SponsorLink", sponsorable, product);
if (Directory.Exists(objDir))
{
foreach (var file in Directory.EnumerateFiles(objDir))
File.Delete(file);
}

Directory.CreateDirectory(objDir);
File.WriteAllText(Path.Combine(objDir, $"{diag.Id}.{diag.Severity}.txt"), diag.GetMessage());
Directory.CreateDirectory(objDir);
File.WriteAllText(Path.Combine(objDir, $"{diag.Id}.{diag.Severity}.txt"), diag.GetMessage());
}
// This is best-effort only. Cases where it can fail, is when bring run from
// the Roslyn analyzer host, which is in program files, and if the project path
// is empty, or in a folder that requires admin rights.
catch (IOException) { }
catch (UnauthorizedAccessException) { }

return diag;
}
Expand Down
48 changes: 42 additions & 6 deletions src/Package/SponsorLinkSettings.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class SponsorLinkSettings
/// </summary>
/// <param name="sponsorable">The sponsor account to check for sponsorships.</param>
/// <param name="product">The product that uses SponsorLink. Used in diagnostics to clarify the product requesting the sponsor link check.</param>
// <Previous release> BACKCOMPAT OVERLOAD -- DO NOT TOUCH
public static SponsorLinkSettings Create(string sponsorable, string product)
=> Create(sponsorable, product,
packageId: default,
Expand All @@ -51,12 +52,13 @@ public static SponsorLinkSettings Create(string sponsorable, string product)
/// a default one is determined from the <paramref name="sponsorable"/> and <paramref name="product"/> values.</param>
/// <param name="pauseMin">Min random milliseconds to apply during build for non-sponsoring users. Use 0 for no pause.</param>
/// <param name="pauseMax">Max random milliseconds to apply during build for non-sponsoring users. Use 0 for no pause.</param>
// <Previous release> BACKCOMPAT OVERLOAD -- DO NOT TOUCH
[EditorBrowsable(EditorBrowsableState.Never)]
public static SponsorLinkSettings Create(string sponsorable, string product,
string? packageId = default,
string? diagnosticsIdPrefix = default,
int pauseMin = 0,
int pauseMax = DefaultMaxPause) => Create(sponsorable, product,
string? packageId,
string? diagnosticsIdPrefix,
int pauseMin,
int pauseMax) => Create(sponsorable, product,
packageId: packageId,
diagnosticsIdPrefix: diagnosticsIdPrefix,
version: default,
Expand All @@ -78,13 +80,46 @@ public static SponsorLinkSettings Create(string sponsorable, string product,
/// <param name="pauseMin">Min random milliseconds to apply during build for non-sponsoring users. Use 0 for no pause.</param>
/// <param name="pauseMax">Max random milliseconds to apply during build for non-sponsoring users. Use 0 for no pause.</param>
/// <param name="quietDays">Optional days to keep warnings quiet so the user has a chance to test the product undisturbed.</param>
// <Previous release> BACKCOMPAT OVERLOAD -- DO NOT TOUCH
[EditorBrowsable(EditorBrowsableState.Never)]
public static SponsorLinkSettings Create(string sponsorable, string product,
string? packageId,
string? version,
string? diagnosticsIdPrefix,
int pauseMin,
int pauseMax,
int? quietDays) => Create(sponsorable, product,
packageId: packageId,
version: version,
diagnosticsIdPrefix: diagnosticsIdPrefix,
pauseMin: pauseMin,
pauseMax: pauseMax,
quietDays: quietDays,
transitive: false);

/// <summary>
/// Creates the settings for <see cref="SponsorLink"/> with the given values.
/// </summary>
/// <param name="sponsorable">The sponsor account to check for sponsorships.</param>
/// <param name="product">The product that uses SponsorLink. Used in diagnostics to clarify the product requesting the sponsor link check.</param>
/// <param name="packageId">Optional NuGet package identifier of the product performing the check. Defaults to <paramref name="product"/>.
/// Used to determine installation time of the product and avoid pausing builds or emitting warnings during the
/// <paramref name="quietDays"/> after install.</param>
/// <param name="version">Optional product or package version.</param>
/// <param name="diagnosticsIdPrefix">Prefix to use for diagnostics with numbers <c>02,03,04</c> reported by default. If not provided,
/// a default one is determined from the <paramref name="sponsorable"/> and <paramref name="product"/> values.</param>
/// <param name="pauseMin">Min random milliseconds to apply during build for non-sponsoring users. Use 0 for no pause.</param>
/// <param name="pauseMax">Max random milliseconds to apply during build for non-sponsoring users. Use 0 for no pause.</param>
/// <param name="quietDays">Optional days to keep warnings quiet so the user has a chance to test the product undisturbed.</param>
/// <param name="transitive">Whether the check is transitive (enforced when dependency is indirect in a project).</param>
public static SponsorLinkSettings Create(string sponsorable, string product,
string? packageId = default,
string? version = default,
string? diagnosticsIdPrefix = default,
int pauseMin = 0,
int pauseMax = DefaultMaxPause,
int? quietDays = default)
int? quietDays = default,
bool transitive = default)
{
if (quietDays < 0)
// Throwing would be a backwards incompatible change.
Expand Down Expand Up @@ -129,7 +164,8 @@ public static SponsorLinkSettings Create(string sponsorable, string product,
PauseMin = pauseMin,
PauseMax = pauseMax,
QuietDays = quietDays,
SupportedDiagnostics = SponsorLink.Diagnostics.GetDescriptors(sponsorable, diagnosticsIdPrefix)
SupportedDiagnostics = SponsorLink.Diagnostics.GetDescriptors(sponsorable, diagnosticsIdPrefix),
Transitive = transitive
};
}

Expand Down

0 comments on commit fdf607d

Please sign in to comment.