Skip to content

Commit

Permalink
Add validation about non-nullable, non-reference record properties
Browse files Browse the repository at this point in the history
[Required] should be used if the non-nullable value will always be set.
  • Loading branch information
joelverhagen committed Aug 30, 2024
1 parent 2a3f1ab commit fb4ab32
Show file tree
Hide file tree
Showing 60 changed files with 350 additions and 145 deletions.
40 changes: 24 additions & 16 deletions src/SourceGenerator/CsvRecordGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace NuGet.Insights
public class CsvRecordGenerator : ISourceGenerator
{
internal const string Category = "NuGet.Insights.SourceGenerator";
private const string InterfaceNamePrefix = "ICsvRecord";
private const string InterfaceNameSuffix = "ICsvRecord";
private const string FullInterfaceName = "NuGet.Insights.ICsvRecord";
private const string FullKustoTypeAttributeName = "NuGet.Insights.KustoTypeAttribute";
private const string NoKustoDDLAttributeName = "NuGet.Insights.NoKustoDDLAttribute";
Expand Down Expand Up @@ -145,16 +145,16 @@ public void Execute(GeneratorExecutionContext context)
// to exist in NuGet.Insights.Worker.Logic and in no other project.
var isTestAssembly = context.Compilation.AssemblyName.EndsWith(".Test", StringComparison.Ordinal);

// System.Diagnostics.Debugger.Launch();
// Debugger.Launch();

var nullable = context.Compilation.GetTypeByMetadataName("System.Nullable`1");
var interfaceType = context.Compilation.GetTypeByMetadataName(FullInterfaceName);
if (interfaceType == null)
{
context.ReportDiagnostic(Diagnostic.Create(
new DiagnosticDescriptor(
id: "EXP0001",
title: $"{InterfaceNamePrefix} interface could not be found",
id: DiagnosticIds.MissingICsvRecordInterface,
title: $"{InterfaceNameSuffix} interface could not be found",
messageFormat: $"The {FullInterfaceName} interface could not be found.",
Category,
DiagnosticSeverity.Error,
Expand All @@ -167,7 +167,7 @@ public void Execute(GeneratorExecutionContext context)
{
context.ReportDiagnostic(Diagnostic.Create(
new DiagnosticDescriptor(
id: "EXP0006",
id: DiagnosticIds.MissingKustoTypeAttribute,
title: $"{FullKustoTypeAttributeName.Split('.').Last()} type could not be found",
messageFormat: $"The {FullKustoTypeAttributeName} type could not be found.",
Category,
Expand All @@ -181,7 +181,7 @@ public void Execute(GeneratorExecutionContext context)
{
context.ReportDiagnostic(Diagnostic.Create(
new DiagnosticDescriptor(
id: "EXP0007",
id: DiagnosticIds.MissingNoKustoDDLAttribute,
title: $"{NoKustoDDLAttributeName.Split('.').Last()} type could not be found",
messageFormat: $"The {NoKustoDDLAttributeName} type could not be found.",
Category,
Expand All @@ -190,10 +190,7 @@ public void Execute(GeneratorExecutionContext context)
Location.None));
}

var propertyVisitorContext = new PropertyVisitorContext(
context,
nullable,
kustoTypeAttributeType);
var requiredAttributeType = context.Compilation.GetTypeByMetadataName("System.ComponentModel.DataAnnotations.RequiredAttribute");

var infos = receiver
.CandidateClasses
Expand Down Expand Up @@ -221,13 +218,13 @@ public void Execute(GeneratorExecutionContext context)
{
context.ReportDiagnostic(Diagnostic.Create(
new DiagnosticDescriptor(
id: "EXP0002",
title: $"{InterfaceNamePrefix} implementor is not partial",
messageFormat: $"The type {{0}} implements {InterfaceNamePrefix} but is not declared as partial.",
id: DiagnosticIds.CsvRecordNotMarkedAsPartial,
title: $"{InterfaceNameSuffix} implementor is not partial",
messageFormat: $"The type {{0}} implements {InterfaceNameSuffix} but is not declared as partial.",
Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true),
Location.Create(info.Candidate.SyntaxTree, info.Candidate.Span),
info.Candidate.GetLocation(),
info.Identifier.Text));
continue;
}
Expand All @@ -246,6 +243,7 @@ public void Execute(GeneratorExecutionContext context)
var kustoTableConstantBuilder = new KustoTableBuilder(indent: 4);
var kustoPartitioningPolicyConstantBuilder = new KustoPartitioningPolicyBuilder(indent: 0, escapeQuotes: true);
var kustoMappingConstantBuilder = new KustoMappingBuilder(indent: 4, escapeQuotes: true);
var validatePropertyNullability = new ValidatePropertyNullability();

var visitors = new IPropertyVisitor[]
{
Expand All @@ -261,6 +259,7 @@ public void Execute(GeneratorExecutionContext context)
kustoTableConstantBuilder,
kustoPartitioningPolicyConstantBuilder,
kustoMappingConstantBuilder,
validatePropertyNullability,
};

var sortedProperties = new List<IPropertySymbol>();
Expand All @@ -278,6 +277,15 @@ public void Execute(GeneratorExecutionContext context)
sortedProperties.Reverse();
var propertyNames = sortedProperties.Select(x => x.Name).ToImmutableHashSet();

var propertyVisitorContext = new PropertyVisitorContext(
context,
info.Candidate.SyntaxTree,
info.Candidate,
hasNoDDLAttribute,
nullable,
kustoTypeAttributeType,
requiredAttributeType);

foreach (var propertySymbol in sortedProperties)
{
var propType = propertySymbol.Type.ToString();
Expand Down Expand Up @@ -313,7 +321,7 @@ public void Execute(GeneratorExecutionContext context)
kustoTableName = kustoTableName.Pluralize();

context.AddSource(
$"{typeName}.{InterfaceNamePrefix}.cs",
$"{typeName}.{InterfaceNameSuffix}.cs",
SourceText.From(
string.Format(
CsvRecordTemplate,
Expand Down Expand Up @@ -399,7 +407,7 @@ private static bool HasInterface(BaseListSyntax baseList)
.Select(x => x.Type)
.OfType<SimpleNameSyntax>()
.Select(x => x.Identifier.Text)
.Any(x => x.IndexOf(InterfaceNamePrefix, StringComparison.OrdinalIgnoreCase) > -1) ?? false;
.Any(x => x.IndexOf(InterfaceNameSuffix, StringComparison.OrdinalIgnoreCase) > -1) ?? false;
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions src/SourceGenerator/DiagnosticIds.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace NuGet.Insights
{
public static class DiagnosticIds
{
public const string MissingICsvRecordInterface = "NI0001";
public const string CsvRecordNotMarkedAsPartial = "NI0002";
public const string MultipleKustoPartioningKeys = "NI0003";
public const string NoKustoPartitioningKeyDefined = "NI0004";
public const string IgnoredKustoPartitioningKey = "NI0005";
public const string MissingKustoTypeAttribute = "NI0006";
public const string MissingNoKustoDDLAttribute = "NI0007";
public const string NonNullablePropertyNotMarkedAsRequired = "NI0008";
}
}
12 changes: 6 additions & 6 deletions src/SourceGenerator/KustoPartitioningPolicyBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ public void OnProperty(PropertyVisitorContext context, IPropertySymbol symbol, s
{
context.GeneratorExecutionContext.ReportDiagnostic(Diagnostic.Create(
new DiagnosticDescriptor(
id: "EXP0005",
id: DiagnosticIds.IgnoredKustoPartitioningKey,
title: "An attribute was marked as both a Kusto partition key and it was ignored.",
messageFormat: "An attribute was marked as both a Kusto partition key and it was ignored.",
CsvRecordGenerator.Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true),
Location.None));
symbol.Locations.FirstOrDefault() ?? Location.None));
return;
}

Expand Down Expand Up @@ -71,13 +71,13 @@ public void OnProperty(PropertyVisitorContext context, IPropertySymbol symbol, s
{
context.GeneratorExecutionContext.ReportDiagnostic(Diagnostic.Create(
new DiagnosticDescriptor(
id: "EXP0003",
id: DiagnosticIds.MultipleKustoPartioningKeys,
title: $"Multiple {AttributeName} attributes were defined on a single type.",
messageFormat: $"Multiple {AttributeName} attributes were defined on a single type.",
CsvRecordGenerator.Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true),
Location.None));
symbol.Locations.FirstOrDefault() ?? Location.None));
return;
}

Expand All @@ -103,13 +103,13 @@ public void Finish(PropertyVisitorContext context)
{
context.GeneratorExecutionContext.ReportDiagnostic(Diagnostic.Create(
new DiagnosticDescriptor(
id: "EXP0004",
id: DiagnosticIds.NoKustoPartitioningKeyDefined,
title: $"No {AttributeName} attributes were defined on a type.",
messageFormat: $"No {AttributeName} attributes were defined on a type.",
CsvRecordGenerator.Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true),
Location.None));
context.TypeDeclarationSyntax.GetLocation()));
return;
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/SourceGenerator/PropertyHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ public static bool IsNullable(PropertyVisitorContext context, IPropertySymbol sy
return false;
}

public static bool IsRequired(PropertyVisitorContext context, IPropertySymbol symbol)
{
var attributeData = symbol
.GetAttributes()
.SingleOrDefault(x => x.AttributeClass.Equals(context.RequiredAttribute, SymbolEqualityComparer.Default));
return attributeData is not null;
}

private const string KustoIgnoreAttributeName = "KustoIgnoreAttribute";

public static bool IsIgnoredInKusto(IPropertySymbol symbol)
Expand Down
15 changes: 14 additions & 1 deletion src/SourceGenerator/PropertyVisitorContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,36 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace NuGet.Insights
{
public class PropertyVisitorContext
{
public PropertyVisitorContext(
GeneratorExecutionContext generatorExecutionContext,
SyntaxTree syntaxTree,
TypeDeclarationSyntax typeDeclarationSyntax,
bool hasNoDDLAttribute,
INamedTypeSymbol nullable,
INamedTypeSymbol kustoTypeAttribute)
INamedTypeSymbol kustoTypeAttribute,
INamedTypeSymbol requiredAttribute)
{
GeneratorExecutionContext = generatorExecutionContext;
SyntaxTree = syntaxTree;
TypeDeclarationSyntax = typeDeclarationSyntax;
HasNoDDLAttribute = hasNoDDLAttribute;
Nullable = nullable;
KustoTypeAttribute = kustoTypeAttribute;
RequiredAttribute = requiredAttribute;
}

public GeneratorExecutionContext GeneratorExecutionContext { get; }
public SyntaxTree SyntaxTree { get; }
public TypeDeclarationSyntax TypeDeclarationSyntax { get; }
public bool HasNoDDLAttribute { get; }
public INamedTypeSymbol Nullable { get; }
public INamedTypeSymbol KustoTypeAttribute { get; }
public INamedTypeSymbol RequiredAttribute { get; }
}
}
41 changes: 41 additions & 0 deletions src/SourceGenerator/ValidatePropertyNullability.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.CodeAnalysis;

namespace NuGet.Insights
{
public class ValidatePropertyNullability : IPropertyVisitor
{
public void OnProperty(PropertyVisitorContext context, IPropertySymbol symbol, string prettyPropType)
{
if (context.HasNoDDLAttribute
|| symbol.Type.IsReferenceType
|| PropertyHelper.IsIgnoredInKusto(symbol)
|| PropertyHelper.IsNullable(context, symbol, out _)
|| PropertyHelper.IsRequired(context, symbol))
{
return;
}

context.GeneratorExecutionContext.ReportDiagnostic(Diagnostic.Create(
new DiagnosticDescriptor(
id: DiagnosticIds.NonNullablePropertyNotMarkedAsRequired,
title: $"Non-nullable property is not marked as [Required].",
messageFormat: $"Non-nullable value type {symbol.ToDisplayString()} is not marked as [Required]. Either make it nullable or mark it as [Required].",
CsvRecordGenerator.Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true),
symbol.Locations.FirstOrDefault() ?? Location.None));
}

public void Finish(PropertyVisitorContext context)
{
}

public string GetResult()
{
throw new NotSupportedException();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.ComponentModel.DataAnnotations;

namespace NuGet.Insights.Worker
{
public enum FileRecordResultType
Expand Down Expand Up @@ -29,6 +31,7 @@ public FileRecord(Guid scanId, DateTimeOffset scanTimestamp, PackageDetailsCatal
ResultType = FileRecordResultType.Available;
}

[Required]
public FileRecordResultType ResultType { get; set; }

public int? SequenceNumber { get; set; }
Expand Down
3 changes: 3 additions & 0 deletions src/Worker.Logic/CatalogScan/ArchiveToCsv/ArchiveEntry.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.ComponentModel.DataAnnotations;

namespace NuGet.Insights.Worker
{
public abstract record ArchiveEntry : PackageRecord
Expand All @@ -21,6 +23,7 @@ public ArchiveEntry(Guid scanId, DateTimeOffset scanTimestamp, PackageDetailsCat
ResultType = ArchiveResultType.Available;
}

[Required]
public ArchiveResultType ResultType { get; set; }

public int? SequenceNumber { get; set; }
Expand Down
3 changes: 3 additions & 0 deletions src/Worker.Logic/CatalogScan/ArchiveToCsv/ArchiveRecord.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.ComponentModel.DataAnnotations;

namespace NuGet.Insights.Worker
{
public abstract record ArchiveRecord : PackageRecord
Expand All @@ -21,6 +23,7 @@ public ArchiveRecord(Guid scanId, DateTimeOffset scanTimestamp, PackageDetailsCa
ResultType = ArchiveResultType.Available;
}

[Required]
public ArchiveResultType ResultType { get; set; }

public long? Size { get; set; }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.ComponentModel.DataAnnotations;

namespace NuGet.Insights.Worker
{
public record PackageRecord
Expand Down Expand Up @@ -45,7 +47,10 @@ public PackageRecord(Guid scanId, DateTimeOffset scanTimestamp, string id, strin

public string Id { get; set; }
public string Version { get; set; }

[Required]
public DateTimeOffset CatalogCommitTimestamp { get; set; }

public DateTimeOffset? Created { get; set; }

public static List<T> Prune<T>(List<T> records, bool isFinalPrune) where T : PackageRecord, IEquatable<T>, IComparable<T>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.ComponentModel.DataAnnotations;
using NuGet.Packaging;

namespace NuGet.Insights.Worker.CatalogDataToCsv
Expand Down Expand Up @@ -55,15 +56,21 @@ private CatalogLeafItemRecord(CatalogLeaf leaf, string pageUrl)
}

public string CommitId { get; set; }

[Required]
public DateTimeOffset CommitTimestamp { get; set; }

public string LowerId { get; set; }

[KustoPartitionKey]
public string Identity { get; set; }

public string Id { get; set; }
public string Version { get; set; }

[Required]
public CatalogLeafType Type { get; set; }

public string Url { get; set; }

public string PageUrl { get; set; }
Expand Down
Loading

0 comments on commit fb4ab32

Please sign in to comment.