C#: Address review comments and make more early returns in Populate.

This commit is contained in:
Michael Nebel
2025-11-07 14:50:20 +01:00
parent d95ebc77ae
commit 7c670cdc3f
23 changed files with 149 additions and 64 deletions

View File

@@ -11,11 +11,12 @@ namespace Semmle.Extraction.CSharp.Entities
public override void Populate(TextWriter trapFile)
{
trapFile.commentblock(this);
if (!Context.OnlyScaffold)
{
WriteLocationToTrap(trapFile.commentblock_location, this, Context.CreateLocation(Symbol.Location));
}
Symbol.CommentLines.ForEach((l, child) => trapFile.commentblock_child(this, l, child));
if (Context.OnlyScaffold)
{
return;
}
WriteLocationToTrap(trapFile.commentblock_location, this, Context.CreateLocation(Symbol.Location));
}
public override bool NeedsPopulation => true;

View File

@@ -21,12 +21,14 @@ namespace Semmle.Extraction.CSharp.Entities
public override void Populate(TextWriter trapFile)
{
location = Context.CreateLocation(Location);
trapFile.commentline(this, Type == CommentLineType.MultilineContinuation ? CommentLineType.Multiline : Type, Text, RawText);
if (!Context.OnlyScaffold)
if (Context.OnlyScaffold)
{
WriteLocationToTrap(trapFile.commentline_location, this, location);
return;
}
location = Context.CreateLocation(Location);
WriteLocationToTrap(trapFile.commentline_location, this, location);
}
public override Microsoft.CodeAnalysis.Location? ReportingLocation => location?.Symbol;

View File

@@ -21,6 +21,11 @@ namespace Semmle.Extraction.CSharp.Entities
protected override void Populate(TextWriter trapFile)
{
if (Context.OnlyScaffold)
{
return;
}
var key = diagnostic.Id;
var messageCount = compilation.messageCounts.AddOrUpdate(key, 1, (_, c) => c + 1);
if (messageCount > limit)

View File

@@ -29,9 +29,17 @@ namespace Semmle.Extraction.CSharp.Entities
ContainingType!.PopulateGenerics();
trapFile.constructors(this, Symbol.ContainingType.Name, ContainingType, (Constructor)OriginalDefinition);
if (Context.ExtractLocation(Symbol) && (!IsDefault || IsBestSourceLocation))
if (Symbol.IsImplicitlyDeclared)
{
WriteLocationToTrap(trapFile.constructor_location, this, Location);
var lineCounts = new LineCounts() { Total = 2, Code = 1, Comment = 0 };
trapFile.numlines(this, lineCounts);
}
ExtractCompilerGenerated(trapFile);
if (Context.OnlyScaffold)
{
return;
}
if (MakeSynthetic)
@@ -40,12 +48,11 @@ namespace Semmle.Extraction.CSharp.Entities
Statements.SyntheticEmptyBlock.Create(Context, this, 0, Location);
}
if (Symbol.IsImplicitlyDeclared)
if (Context.ExtractLocation(Symbol) && (!IsDefault || IsBestSourceLocation))
{
var lineCounts = new LineCounts() { Total = 2, Code = 1, Comment = 0 };
trapFile.numlines(this, lineCounts);
WriteLocationToTrap(trapFile.constructor_location, this, Location);
}
ExtractCompilerGenerated(trapFile);
}
protected override void ExtractInitializers(TextWriter trapFile)

View File

@@ -15,6 +15,7 @@ namespace Semmle.Extraction.CSharp.Entities
ContainingType!.PopulateGenerics();
trapFile.destructors(this, $"~{Symbol.ContainingType.Name}", ContainingType, OriginalDefinition(Context, this, Symbol));
if (Context.ExtractLocation(Symbol))
{
WriteLocationToTrap(trapFile.destructor_location, this, Location);

View File

@@ -37,7 +37,6 @@ namespace Semmle.Extraction.CSharp.Entities
Method.Create(Context, remover);
PopulateModifiers(trapFile);
BindComments();
var declSyntaxReferences = IsSourceDeclaration
? Symbol.DeclaringSyntaxReferences.Select(d => d.GetSyntax()).ToArray()
@@ -51,6 +50,13 @@ namespace Semmle.Extraction.CSharp.Entities
TypeMention.Create(Context, syntax.ExplicitInterfaceSpecifier!.Name, this, explicitInterface);
}
if (Context.OnlyScaffold)
{
return;
}
BindComments();
if (Context.ExtractLocation(Symbol))
{
WriteLocationsToTrap(trapFile.event_location, this, Locations);

View File

@@ -28,6 +28,11 @@ namespace Semmle.Extraction.CSharp.Entities
protected override void Populate(TextWriter trapFile)
{
if (Context.OnlyScaffold)
{
return;
}
// For the time being we're counting the number of messages per severity, we could introduce other groupings in the future
var key = msg.Severity.ToString();
groupedMessageCounts.AddOrUpdate(key, 1, (_, c) => c + 1);

View File

@@ -49,12 +49,17 @@ namespace Semmle.Extraction.CSharp.Entities
}
}
if (Context.OnlyScaffold)
{
return;
}
if (Context.ExtractLocation(Symbol))
{
WriteLocationsToTrap(trapFile.field_location, this, Locations);
}
if (!IsSourceDeclaration || !Symbol.FromSource() || Context.OnlyScaffold)
if (!IsSourceDeclaration || !Symbol.FromSource())
return;
Context.BindComments(this, Location.Symbol);

View File

@@ -19,10 +19,6 @@ namespace Semmle.Extraction.CSharp.Entities
var type = Type.Create(Context, Symbol.Type);
trapFile.indexers(this, Symbol.GetName(useMetadataName: true), ContainingType!, type.TypeRef, OriginalDefinition);
if (Context.ExtractLocation(Symbol))
{
WriteLocationsToTrap(trapFile.indexer_location, this, Locations);
}
var getter = BodyDeclaringSymbol.GetMethod;
var setter = BodyDeclaringSymbol.SetMethod;
@@ -42,27 +38,9 @@ namespace Semmle.Extraction.CSharp.Entities
Parameter.Create(Context, Symbol.Parameters[i], this, original);
}
if (IsSourceDeclaration && !Context.OnlyScaffold)
{
var expressionBody = ExpressionBody;
if (expressionBody is not null)
{
// The expression may need to reference parameters in the getter.
// So we need to arrange that the expression is populated after the getter.
Context.PopulateLater(() => Expression.CreateFromNode(new ExpressionNodeInfo(Context, expressionBody, this, 0).SetType(Symbol.GetAnnotatedType())));
}
}
PopulateAttributes();
PopulateModifiers(trapFile);
if (Context.OnlyScaffold)
{
return;
}
BindComments();
var declSyntaxReferences = IsSourceDeclaration
? Symbol.DeclaringSyntaxReferences.
Select(d => d.GetSyntax()).OfType<IndexerDeclarationSyntax>().ToArray()
@@ -76,6 +54,28 @@ namespace Semmle.Extraction.CSharp.Entities
TypeMention.Create(Context, syntax.ExplicitInterfaceSpecifier!.Name, this, explicitInterface);
}
if (Context.OnlyScaffold)
{
return;
}
if (Context.ExtractLocation(Symbol))
{
WriteLocationsToTrap(trapFile.indexer_location, this, Locations);
}
if (IsSourceDeclaration)
{
var expressionBody = ExpressionBody;
if (expressionBody is not null)
{
// The expression may need to reference parameters in the getter.
// So we need to arrange that the expression is populated after the getter.
Context.PopulateLater(() => Expression.CreateFromNode(new ExpressionNodeInfo(Context, expressionBody, this, 0).SetType(Symbol.GetAnnotatedType())));
}
}
BindComments();
foreach (var syntax in declSyntaxReferences)
TypeMention.Create(Context, syntax.Type, this, type);

View File

@@ -41,6 +41,11 @@ namespace Semmle.Extraction.CSharp.Entities
trapFile.localvars(this, Kinds.VariableKind.None, Symbol.Name, @var, Type.Create(Context, parent.Type).TypeRef, parent);
}
if (Context.OnlyScaffold)
{
return;
}
WriteLocationToTrap(trapFile.localvar_location, this, Location);
DefineConstantValue(trapFile);

View File

@@ -94,7 +94,7 @@ namespace Semmle.Extraction.CSharp.Entities
{
trapFile.explicitly_implements(this, explicitInterface.TypeRef);
if (IsSourceDeclaration && !Context.OnlyScaffold)
if (IsSourceDeclaration)
{
foreach (var syntax in Symbol.DeclaringSyntaxReferences.Select(d => d.GetSyntax()).OfType<MethodDeclarationSyntax>())
TypeMention.Create(Context, syntax.ExplicitInterfaceSpecifier!.Name, this, explicitInterface);

View File

@@ -35,7 +35,6 @@ namespace Semmle.Extraction.CSharp.Entities
var ns = Namespace.Create(Context, @namespace);
trapFile.namespace_declarations(this, ns);
WriteLocationToTrap(trapFile.namespace_declaration_location, this, Context.CreateLocation(node.Name.GetLocation()));
var visitor = new Populators.TypeOrNamespaceVisitor(Context, trapFile, this);
@@ -48,6 +47,12 @@ namespace Semmle.Extraction.CSharp.Entities
{
trapFile.parent_namespace_declaration(this, parent);
}
if (Context.OnlyScaffold)
{
return;
}
WriteLocationToTrap(trapFile.namespace_declaration_location, this, Context.CreateLocation(node.Name.GetLocation()));
}
public static NamespaceDeclaration Create(Context cx, BaseNamespaceDeclarationSyntax decl, NamespaceDeclaration parent)

View File

@@ -34,7 +34,17 @@ namespace Semmle.Extraction.CSharp.Entities
var returnType = Type.Create(Context, Symbol.ReturnType);
trapFile.methods(this, Name, ContainingType, returnType.TypeRef, OriginalDefinition);
if (IsSourceDeclaration && !Context.OnlyScaffold)
PopulateGenerics(trapFile);
Overrides(trapFile);
ExtractRefReturn(trapFile, Symbol, this);
ExtractCompilerGenerated(trapFile);
if (Context.OnlyScaffold)
{
return;
}
if (IsSourceDeclaration)
{
foreach (var declaration in Symbol.DeclaringSyntaxReferences.Select(s => s.GetSyntax()).OfType<MethodDeclarationSyntax>())
{
@@ -47,11 +57,6 @@ namespace Semmle.Extraction.CSharp.Entities
{
WriteLocationsToTrap(trapFile.method_location, this, Locations);
}
PopulateGenerics(trapFile);
Overrides(trapFile);
ExtractRefReturn(trapFile, Symbol, this);
ExtractCompilerGenerated(trapFile);
}
private bool IsCompilerGeneratedDelegate() =>

View File

@@ -115,6 +115,11 @@ namespace Semmle.Extraction.CSharp.Entities
var type = Type.Create(Context, Symbol.Type);
trapFile.@params(this, Name, type.TypeRef, Ordinal, ParamKind, Parent!, Original);
if (Context.OnlyScaffold)
{
return;
}
if (Context.ExtractLocation(Symbol))
{
var locations = Context.GetLocations(Symbol);
@@ -140,7 +145,7 @@ namespace Semmle.Extraction.CSharp.Entities
Context.PopulateLater(defaultValueExpressionCreation);
}
if (!IsSourceDeclaration || !Symbol.FromSource() || Context.OnlyScaffold)
if (!IsSourceDeclaration || !Symbol.FromSource())
return;
BindComments();

View File

@@ -13,10 +13,15 @@ namespace Semmle.Extraction.CSharp.Entities
PopulatePreprocessor(trapFile);
trapFile.preprocessor_directive_active(this, Symbol.IsActive);
WriteLocationToTrap(trapFile.preprocessor_directive_location, this, Context.CreateLocation(ReportingLocation));
var compilation = Compilation.Create(Context);
trapFile.preprocessor_directive_compilation(this, compilation);
if (Context.OnlyScaffold)
{
return;
}
WriteLocationToTrap(trapFile.preprocessor_directive_location, this, Context.CreateLocation(ReportingLocation));
}
protected abstract void PopulatePreprocessor(TextWriter trapFile);

View File

@@ -40,7 +40,6 @@ namespace Semmle.Extraction.CSharp.Entities
{
PopulateAttributes();
PopulateModifiers(trapFile);
BindComments();
PopulateNullability(trapFile, Symbol.GetAnnotatedType());
PopulateRefKind(trapFile, Symbol.RefKind);
@@ -69,12 +68,19 @@ namespace Semmle.Extraction.CSharp.Entities
TypeMention.Create(Context, syntax.ExplicitInterfaceSpecifier!.Name, this, explicitInterface);
}
if (Context.OnlyScaffold)
{
return;
}
BindComments();
if (Context.ExtractLocation(Symbol))
{
WriteLocationsToTrap(trapFile.property_location, this, Locations);
}
if (IsSourceDeclaration && Symbol.FromSource() && !Context.OnlyScaffold)
if (IsSourceDeclaration && Symbol.FromSource())
{
var expressionBody = ExpressionBody;
if (expressionBody is not null)

View File

@@ -59,6 +59,11 @@ namespace Semmle.Extraction.CSharp.Entities
protected override void Populate(TextWriter trapFile)
{
if (Context.OnlyScaffold)
{
return;
}
switch (syntax.Kind())
{
case SyntaxKind.ArrayType:

View File

@@ -16,10 +16,14 @@ namespace Semmle.Extraction.CSharp.Entities
public override void Populate(TextWriter trapFile)
{
trapFile.types(this, Kinds.TypeKind.DYNAMIC, "dynamic");
WriteLocationToTrap(trapFile.type_location, this, Location);
trapFile.has_modifiers(this, Modifier.Create(Context, "public"));
trapFile.parent_namespace(this, Namespace.Create(Context, Context.Compilation.GlobalNamespace));
if (Context.OnlyScaffold)
{
return;
}
WriteLocationToTrap(trapFile.type_location, this, Location);
}
public override void WriteId(EscapingTextWriter trapFile)

View File

@@ -51,6 +51,10 @@ namespace Semmle.Extraction.CSharp.Entities
trapFile.tuple_element(this, index++, element);
}
if (Context.OnlyScaffold)
{
return;
}
// Note: symbol.Locations seems to be very inconsistent
// about what locations are available for a tuple type.
// Sometimes it's the source code, and sometimes it's empty.

View File

@@ -26,13 +26,18 @@ namespace Semmle.Extraction.CSharp.Entities
var parentNs = Namespace.Create(Context, Symbol.TypeParameterKind == TypeParameterKind.Method ? Context.Compilation.GlobalNamespace : Symbol.ContainingNamespace);
trapFile.parent_namespace(this, parentNs);
if (Context.OnlyScaffold)
{
return;
}
if (Context.ExtractLocation(Symbol))
{
var locations = Context.GetLocations(Symbol);
WriteLocationsToTrap(trapFile.type_location, this, locations);
}
if (IsSourceDeclaration && !Context.OnlyScaffold)
if (IsSourceDeclaration)
{
var declSyntaxReferences = Symbol.DeclaringSyntaxReferences
.Select(d => d.GetSyntax())

View File

@@ -26,12 +26,20 @@ namespace Semmle.Extraction.CSharp.Entities
returnType.TypeRef,
(UserOperator)OriginalDefinition);
ContainingType.PopulateGenerics();
Overrides(trapFile);
if (Context.OnlyScaffold)
{
return;
}
if (Context.ExtractLocation(Symbol))
{
WriteLocationsToTrap(trapFile.operator_location, this, Locations);
}
if (IsSourceDeclaration && !Context.OnlyScaffold)
if (IsSourceDeclaration)
{
var declSyntaxReferences = Symbol.DeclaringSyntaxReferences.Select(s => s.GetSyntax()).ToArray();
foreach (var declaration in declSyntaxReferences.OfType<OperatorDeclarationSyntax>())
@@ -39,9 +47,6 @@ namespace Semmle.Extraction.CSharp.Entities
foreach (var declaration in declSyntaxReferences.OfType<ConversionOperatorDeclarationSyntax>())
TypeMention.Create(Context, declaration.Type, this, returnType);
}
ContainingType.PopulateGenerics();
Overrides(trapFile);
}
public override bool NeedsPopulation => Context.Defines(Symbol) || IsImplicitOperator(out _);

View File

@@ -20,6 +20,11 @@ namespace Semmle.Extraction.CSharp.Entities
protected override void Populate(TextWriter trapFile)
{
if (Context.OnlyScaffold)
{
return;
}
// This is guaranteed to be non-null as we only deal with "using namespace" not "using X = Y"
var name = node.Name!;
@@ -32,10 +37,7 @@ namespace Semmle.Extraction.CSharp.Entities
{
var ns = Namespace.Create(Context, namespaceSymbol);
trapFile.using_namespace_directives(this, ns);
if (!Context.OnlyScaffold)
{
trapFile.using_directive_location(this, Context.CreateLocation(ReportingLocation));
}
trapFile.using_directive_location(this, Context.CreateLocation(ReportingLocation));
}
else
{
@@ -49,10 +51,7 @@ namespace Semmle.Extraction.CSharp.Entities
// A "using static"
var m = Type.Create(Context, (ITypeSymbol?)info.Symbol);
trapFile.using_static_directives(this, m.TypeRef);
if (!Context.OnlyScaffold)
{
trapFile.using_directive_location(this, Context.CreateLocation(ReportingLocation));
}
trapFile.using_directive_location(this, Context.CreateLocation(ReportingLocation));
}
if (node.GlobalKeyword.IsKind(SyntaxKind.GlobalKeyword))

View File

@@ -68,7 +68,7 @@ namespace Semmle.Extraction.CSharp
/// ]
/// }
/// </summary>
public record ChangedFiles
private record ChangedFiles
{
public string[]? Changes { get; set; }
}