Skip to content

C#: Only extract public and protected members from metadata. #18159

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.

Sign up for GitHub

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

Merged
michaelnebel merged 6 commits into from
Dec 4, 2024
Merged

C#: Only extract public and protected members from metadata. #18159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f7874c3
C#: Only extract Public and Protected members from reference assemblies.
michaelnebel Nov 26, 2024
a09262b
C#: Update expected test output.
michaelnebel Nov 29, 2024
27d2f9e
C#: Add change-note.
michaelnebel Dec 3, 2024
4675426
C#: Update change note with info on private fields.
michaelnebel Dec 3, 2024
6928f7d
C#: Address review comment.
michaelnebel Dec 3, 2024
395bdd0
C#: Address more review comments.
michaelnebel Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,5 +642,40 @@ public static AnnotatedTypeSymbol GetType(this Context cx, Microsoft.CodeAnalysi
/// </summary>
public static IEnumerable<AnnotatedTypeSymbol> GetAnnotatedTypeArguments(this INamedTypeSymbol symbol) =>
symbol.TypeArguments.Zip(symbol.TypeArgumentNullableAnnotations, (t, a) => new AnnotatedTypeSymbol(t, a));

/// <summary>
/// Returns true if the symbol is public, protected or protected internal.
/// </summary>
public static bool IsPublicOrProtected(this ISymbol symbol) =>
symbol.DeclaredAccessibility == Accessibility.Public
|| symbol.DeclaredAccessibility == Accessibility.Protected
|| symbol.DeclaredAccessibility == Accessibility.ProtectedOrInternal;

/// <summary>
/// Returns true if the given symbol should be extracted.
/// </summary>
public static bool ShouldExtractSymbol(this ISymbol symbol)
{
// Extract all source symbols and public/protected metadata symbols.
if (symbol.Locations.Any(x => !x.IsInMetadata) || symbol.IsPublicOrProtected())
{
return true;
}
if (symbol is IMethodSymbol method)
{
return method.ExplicitInterfaceImplementations.Any(m => m.ContainingType.ShouldExtractSymbol());
}
if (symbol is IPropertySymbol property)
{
return property.ExplicitInterfaceImplementations.Any(m => m.ContainingType.ShouldExtractSymbol());
}
return false;
}

/// <summary>
/// Returns the symbols that should be extracted.
/// </summary>
public static IEnumerable<T> ExtractionCandidates<T>(this IEnumerable<T> symbols) where T : ISymbol =>
symbols.Where(symbol => symbol.ShouldExtractSymbol());
}
}
2 changes: 1 addition & 1 deletion csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public void Overrides(TextWriter trapFile)
}
}

if (Symbol.OverriddenMethod is not null)
if (Symbol.OverriddenMethod is not null && Symbol.OverriddenMethod.ShouldExtractSymbol())
{
trapFile.overrides(this, Method.Create(Context, Symbol.OverriddenMethod));
}
Expand Down
12 changes: 6 additions & 6 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ private void ExtractParametersForDelegateLikeType(TextWriter trapFile, IMethodSy
}

/// <summary>
/// Called to extract all members and nested types.
/// Called to extract members and nested types.
/// This is called on each member of a namespace,
/// in either source code or an assembly.
/// </summary>
Expand All @@ -236,7 +236,7 @@ public void ExtractRecursive()
Context.BindComments(this, l);
}

foreach (var member in Symbol.GetMembers())
foreach (var member in Symbol.GetMembers().ExtractionCandidates())
{
switch (member.Kind)
{
Expand All @@ -262,16 +262,16 @@ public void PopulateGenerics()

var members = new List<ISymbol>();

foreach (var member in Symbol.GetMembers())
foreach (var member in Symbol.GetMembers().ExtractionCandidates())
members.Add(member);
foreach (var member in Symbol.GetTypeMembers())
foreach (var member in Symbol.GetTypeMembers().ExtractionCandidates())
members.Add(member);

// Mono extractor puts all BASE interface members as members of the current interface.

if (Symbol.TypeKind == TypeKind.Interface)
{
foreach (var baseInterface in Symbol.Interfaces)
foreach (var baseInterface in Symbol.Interfaces.ExtractionCandidates())
{
foreach (var member in baseInterface.GetMembers())
members.Add(member);
Expand All @@ -288,7 +288,7 @@ public void PopulateGenerics()
if (Symbol.BaseType is not null)
Create(Context, Symbol.BaseType).PopulateGenerics();

foreach (var i in Symbol.Interfaces)
foreach (var i in Symbol.Interfaces.ExtractionCandidates())
{
Create(Context, i).PopulateGenerics();
}
Expand Down
2 changes: 1 addition & 1 deletion csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ private static void AnalyseNamespace(Context cx, INamespaceSymbol ns)
AnalyseNamespace(cx, memberNamespace);
}

foreach (var memberType in ns.GetTypeMembers())
foreach (var memberType in ns.GetTypeMembers().ExtractionCandidates())
{
Entities.Type.Create(cx, memberType).ExtractRecursive();
}
Expand Down
1 change: 0 additions & 1 deletion csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,6 @@ private static ExitCode AnalyseTracing(
compilerArguments.CompilationOptions
.WithAssemblyIdentityComparer(DesktopAssemblyIdentityComparer.Default)
.WithStrongNameProvider(new DesktopStrongNameProvider(compilerArguments.KeyFileSearchPaths))
.WithMetadataImportOptions(MetadataImportOptions.All)
);
},
(compilation, options) => analyser.EndInitialize(compilerArguments, options, compilation, cwd, args),
Expand Down
4 changes: 4 additions & 0 deletions csharp/ql/lib/change-notes/2024-12-03-public-protected-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Only extract *public* and *protected* members from reference assemblies. This yields an approximate average speed-up of around 10% for extraction and query execution. Custom MaD rows using `Field`-based summaries may need to be changed to `SyntheticField`-based flows if they reference private fields.
3 changes: 0 additions & 3 deletions csharp/ql/test/library-tests/conversion/operator/Operator.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,13 @@
| Int32 | Decimal |
| Int32 | Index |
| Int32 | Int128 |
| Int32 | MetadataToken |
| Int32 | NFloat |
| Int64 | Decimal |
| Int64 | Int128 |
| Int64 | NFloat |
| IntPtr | Int128 |
| IntPtr | NFloat |
| Memory`1 | ReadOnlyMemory<T> |
| MetadataToken | Int32 |
| NFloat | Double |
| SByte | Decimal |
| SByte | Half |
Expand Down Expand Up @@ -59,4 +57,3 @@
| UIntPtr | Int128 |
| UIntPtr | NFloat |
| UIntPtr | UInt128 |
| UnixFileMode | Nullable<UnixFileMode> |
6 changes: 0 additions & 6 deletions csharp/ql/test/library-tests/conversion/reftype/RefType.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,6 @@
| IReadOnlyList<T4> | dynamic |
| Int16[] | Object |
| Int16[] | dynamic |
| Int32[,] | Object |
| Int32[,] | dynamic |
| Int32[] | Object |
| Int32[] | dynamic |
| Int64[] | Object |
Expand Down Expand Up @@ -223,8 +221,6 @@
| T5 | C1 |
| T5 | Object |
| T5 | dynamic |
| UInt16[][] | Object |
| UInt16[][] | dynamic |
| UInt32[] | Object |
| UInt32[] | dynamic |
| UInt64[] | Object |
Expand Down Expand Up @@ -283,7 +279,6 @@
| null | IReadOnlyList<T3> |
| null | IReadOnlyList<T4> |
| null | Int16[] |
| null | Int32[,] |
| null | Int32[] |
| null | Int64[] |
| null | Object |
Expand All @@ -294,7 +289,6 @@
| null | T4 |
| null | T4[] |
| null | T5 |
| null | UInt16[][] |
| null | UInt32[] |
| null | UInt64[] |
| null | dynamic |
35 changes: 0 additions & 35 deletions csharp/ql/test/library-tests/csharp9/FunctionPointer.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,68 +1,33 @@
type
| file://:0:0:0:0 | delegate* default<A,B> | B | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<B,A> | A | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<Byte ref,Void> | Void | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | ref int | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 ref readonly> | readonly int | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<Int32*,Void*> | Void* | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<Int32> | int | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<IntPtr,Byte ref,PortableTailCallFrame*,Void> | Void | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<Object,Void> | Void | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<T,Int32> | int | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<Void*,Int32*> | int* | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* default<Void*,Object> | object | DefaultCallingConvention |
| file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | Void | StdCallCallingConvention |
| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | Void | UnmanagedCallingConvention |
| file://:0:0:0:0 | delegate* unmanaged<Char*,IntPtr,Void> | Void | UnmanagedCallingConvention |
| file://:0:0:0:0 | delegate* unmanaged<Int32,PosixSignal,Int32> | int | UnmanagedCallingConvention |
| file://:0:0:0:0 | delegate* unmanaged<IntPtr,Int32> | int | UnmanagedCallingConvention |
| file://:0:0:0:0 | delegate* unmanaged<IntPtr,Void> | Void | UnmanagedCallingConvention |
| file://:0:0:0:0 | delegate* unmanaged<NoGCRegionCallbackFinalizerWorkItem*,Void> | Void | UnmanagedCallingConvention |
| file://:0:0:0:0 | delegate* unmanaged<Void*,Byte*,Void> | Void | UnmanagedCallingConvention |
| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | Void | UnmanagedCallingConvention |
| file://:0:0:0:0 | delegate* unmanaged<Void> | Void | UnmanagedCallingConvention |
unmanagedCallingConvention
parameter
| file://:0:0:0:0 | delegate* default<A,B> | 0 | file://:0:0:0:0 | | A |
| file://:0:0:0:0 | delegate* default<B,A> | 0 | file://:0:0:0:0 | | B |
| file://:0:0:0:0 | delegate* default<Byte ref,Void> | 0 | file://:0:0:0:0 | | ref byte! |
| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | 0 | file://:0:0:0:0 | | ref int! |
| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | 1 | file://:0:0:0:0 | `1 | out object? |
| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | 2 | file://:0:0:0:0 | `2 | readonly int! |
| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 ref readonly> | 0 | file://:0:0:0:0 | | ref int! |
| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 ref readonly> | 1 | file://:0:0:0:0 | `1 | out object? |
| file://:0:0:0:0 | delegate* default<Int32*,Void*> | 0 | file://:0:0:0:0 | | int*! |
| file://:0:0:0:0 | delegate* default<IntPtr,Byte ref,PortableTailCallFrame*,Void> | 0 | file://:0:0:0:0 | | IntPtr! |
| file://:0:0:0:0 | delegate* default<IntPtr,Byte ref,PortableTailCallFrame*,Void> | 1 | file://:0:0:0:0 | `1 | ref byte! |
| file://:0:0:0:0 | delegate* default<IntPtr,Byte ref,PortableTailCallFrame*,Void> | 2 | file://:0:0:0:0 | `2 | PortableTailCallFrame*! |
| file://:0:0:0:0 | delegate* default<Object,Void> | 0 | file://:0:0:0:0 | | object |
| file://:0:0:0:0 | delegate* default<T,Int32> | 0 | file://:0:0:0:0 | | T |
| file://:0:0:0:0 | delegate* default<Void*,Int32*> | 0 | file://:0:0:0:0 | | Void*! |
| file://:0:0:0:0 | delegate* default<Void*,Object> | 0 | file://:0:0:0:0 | | Void*! |
| file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | 0 | file://:0:0:0:0 | | ref int! |
| file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | 1 | file://:0:0:0:0 | `1 | out object? |
| file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | 2 | file://:0:0:0:0 | `2 | T |
| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 0 | file://:0:0:0:0 | | byte*! |
| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 1 | file://:0:0:0:0 | `1 | int! |
| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 2 | file://:0:0:0:0 | `2 | byte! |
| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 3 | file://:0:0:0:0 | `3 | long! |
| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 4 | file://:0:0:0:0 | `4 | long! |
| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 5 | file://:0:0:0:0 | `5 | EVENT_FILTER_DESCRIPTOR*! |
| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 6 | file://:0:0:0:0 | `6 | Void*! |
| file://:0:0:0:0 | delegate* unmanaged<Char*,IntPtr,Void> | 0 | file://:0:0:0:0 | | char*! |
| file://:0:0:0:0 | delegate* unmanaged<Char*,IntPtr,Void> | 1 | file://:0:0:0:0 | `1 | IntPtr! |
| file://:0:0:0:0 | delegate* unmanaged<Int32,PosixSignal,Int32> | 0 | file://:0:0:0:0 | | int! |
| file://:0:0:0:0 | delegate* unmanaged<Int32,PosixSignal,Int32> | 1 | file://:0:0:0:0 | `1 | PosixSignal! |
| file://:0:0:0:0 | delegate* unmanaged<IntPtr,Int32> | 0 | file://:0:0:0:0 | | IntPtr! |
| file://:0:0:0:0 | delegate* unmanaged<IntPtr,Void> | 0 | file://:0:0:0:0 | | IntPtr! |
| file://:0:0:0:0 | delegate* unmanaged<NoGCRegionCallbackFinalizerWorkItem*,Void> | 0 | file://:0:0:0:0 | | NoGCRegionCallbackFinalizerWorkItem*! |
| file://:0:0:0:0 | delegate* unmanaged<Void*,Byte*,Void> | 0 | file://:0:0:0:0 | | Void*! |
| file://:0:0:0:0 | delegate* unmanaged<Void*,Byte*,Void> | 1 | file://:0:0:0:0 | `1 | byte*! |
| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 0 | file://:0:0:0:0 | | Void*! |
| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 1 | file://:0:0:0:0 | `1 | Void*! |
| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 2 | file://:0:0:0:0 | `2 | Void*! |
| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 3 | file://:0:0:0:0 | `3 | GCConfigurationType! |
| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 4 | file://:0:0:0:0 | `4 | long! |
invocation
| FunctionPointer.cs:17:21:17:43 | function pointer call |
| FunctionPointer.cs:23:13:23:44 | function pointer call |
Expand Down
Loading