Skip to content

Commit 1b769eb

Browse files
committed
C#: Address more review comments
1 parent 51dc151 commit 1b769eb

11 files changed

Lines changed: 67 additions & 139 deletions

File tree

csharp/extractor/Semmle.Extraction.CSharp/Entities/Modifier.cs

Lines changed: 9 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,45 +5,24 @@
55

66
namespace Semmle.Extraction.CSharp.Entities
77
{
8-
/// <summary>
9-
/// Provide a "Key" object to allow modifiers to exist as entities in the extractor
10-
/// hash map. (Raw strings would work as keys but might clash with other types).
11-
/// </summary>
12-
class ModifierKey : Object
8+
class Modifier : Extraction.CachedEntity<string>
139
{
14-
public readonly string name;
15-
16-
public ModifierKey(string m)
17-
{
18-
name = m;
19-
}
20-
21-
public override bool Equals(Object obj)
22-
{
23-
return obj.GetType() == GetType() && name == ((ModifierKey)obj).name;
24-
}
25-
26-
public override int GetHashCode() => 13 * name.GetHashCode();
27-
}
28-
29-
class Modifier : Extraction.CachedEntity<ModifierKey>
30-
{
31-
Modifier(Context cx, ModifierKey init)
10+
Modifier(Context cx, string init)
3211
: base(cx, init) { }
3312

3413
public override Microsoft.CodeAnalysis.Location ReportingLocation => null;
3514

3615
public override void WriteId(TextWriter trapFile)
3716
{
38-
trapFile.Write(symbol.name);
17+
trapFile.Write(symbol);
3918
trapFile.Write(";modifier");
4019
}
4120

4221
public override bool NeedsPopulation => true;
4322

4423
public override void Populate(TextWriter trapFile)
4524
{
46-
trapFile.modifiers(Label, symbol.name);
25+
trapFile.modifiers(Label, symbol);
4726
}
4827

4928
public static string AccessbilityModifier(Accessibility access)
@@ -154,21 +133,20 @@ public static void ExtractModifiers(Context cx, TextWriter trapFile, IEntity key
154133

155134
public static Modifier Create(Context cx, string modifier)
156135
{
157-
var modifierKey = new ModifierKey(modifier);
158-
return ModifierFactory.Instance.CreateEntity(cx, modifierKey, modifierKey);
136+
return ModifierFactory.Instance.CreateEntity(cx, (typeof(Modifier), modifier), modifier);
159137
}
160138

161139
public static Modifier Create(Context cx, Accessibility access)
162140
{
163-
var modifierKey = new ModifierKey(AccessbilityModifier(access));
164-
return ModifierFactory.Instance.CreateEntity(cx, modifierKey, modifierKey);
141+
var modifier = AccessbilityModifier(access);
142+
return ModifierFactory.Instance.CreateEntity(cx, (typeof(Modifier), modifier), modifier);
165143
}
166144

167-
class ModifierFactory : ICachedEntityFactory<ModifierKey, Modifier>
145+
class ModifierFactory : ICachedEntityFactory<string, Modifier>
168146
{
169147
public static readonly ModifierFactory Instance = new ModifierFactory();
170148

171-
public Modifier Create(Context cx, ModifierKey init) => new Modifier(cx, init);
149+
public Modifier Create(Context cx, string init) => new Modifier(cx, init);
172150
}
173151
public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.OptionalLabel;
174152
}

csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,7 @@ public override bool Equals(object obj)
202202
return obj != null && obj.GetType() == typeof(VarargsType);
203203
}
204204

205-
static readonly object cacheKey = new object();
206-
public static VarargsType Create(Context cx) => VarargsTypeFactory.Instance.CreateEntity(cx, cacheKey, null);
205+
public static VarargsType Create(Context cx) => VarargsTypeFactory.Instance.CreateEntity(cx, typeof(VarargsType), null);
207206

208207
class VarargsTypeFactory : ICachedEntityFactory<string, VarargsType>
209208
{
@@ -238,8 +237,7 @@ public override bool Equals(object obj)
238237
return obj != null && obj.GetType() == typeof(VarargsParam);
239238
}
240239

241-
static readonly object cacheKey = new object();
242-
public static VarargsParam Create(Context cx, Method method) => VarargsParamFactory.Instance.CreateEntity(cx, cacheKey, method);
240+
public static VarargsParam Create(Context cx, Method method) => VarargsParamFactory.Instance.CreateEntity(cx, typeof(VarargsParam), method);
243241

244242
class VarargsParamFactory : ICachedEntityFactory<Method, VarargsParam>
245243
{
@@ -266,27 +264,8 @@ public override void Populate(TextWriter trapFile)
266264
trapFile.param_location(this, Original.Location);
267265
}
268266

269-
sealed class ConstructedExtensionParameterCacheKey
270-
{
271-
public readonly IParameterSymbol Parameter;
272-
public readonly ITypeSymbol ConstructedType;
273-
public ConstructedExtensionParameterCacheKey(IParameterSymbol parameter, ITypeSymbol constructedType)
274-
{
275-
Parameter = parameter;
276-
ConstructedType = constructedType;
277-
}
278-
279-
public override int GetHashCode() =>
280-
(Parameter, ConstructedType).GetHashCode();
281-
282-
public override bool Equals(object obj) =>
283-
obj is ConstructedExtensionParameterCacheKey k &&
284-
SymbolEqualityComparer.Default.Equals(k.Parameter, Parameter) &&
285-
SymbolEqualityComparer.Default.Equals(k.ConstructedType, ConstructedType);
286-
}
287-
288267
public static ConstructedExtensionParameter Create(Context cx, Method method, Parameter parameter) =>
289-
ExtensionParamFactory.Instance.CreateEntity(cx, new ConstructedExtensionParameterCacheKey(parameter.symbol, method.symbol.ReceiverType), (method, parameter));
268+
ExtensionParamFactory.Instance.CreateEntity(cx, (new SymbolEqualityWrapper(parameter.symbol), new SymbolEqualityWrapper(method.symbol.ReceiverType)), (method, parameter));
290269

291270
class ExtensionParamFactory : ICachedEntityFactory<(Method, Parameter), ConstructedExtensionParameter>
292271
{

csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using Microsoft.CodeAnalysis;
23
using Microsoft.CodeAnalysis.CSharp.Syntax;
34
using Semmle.Extraction.CSharp.Entities.Expressions;
@@ -11,9 +12,13 @@ namespace Semmle.Extraction.CSharp.Entities
1112
class Property : CachedSymbol<IPropertySymbol>, IExpressionParentEntity
1213
{
1314
protected Property(Context cx, IPropertySymbol init)
14-
: base(cx, init) { }
15+
: base(cx, init)
16+
{
17+
type = new Lazy<Type>(() => Type.Create(Context, symbol.Type));
18+
}
1519

16-
Type Type => Type.Create(Context, symbol.Type);
20+
readonly Lazy<Type> type;
21+
Type Type => type.Value;
1722

1823
public override void WriteId(TextWriter trapFile)
1924
{

csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,25 @@ public override IEnumerable<Extraction.Entities.Location> Locations
107107

108108
public override Microsoft.CodeAnalysis.Location ReportingLocation => GetLocations(symbol).FirstOrDefault();
109109

110+
bool IsAnonymousType() => symbol.IsAnonymousType || symbol.Name.Contains("__AnonymousType");
111+
110112
public override void WriteId(TextWriter trapFile)
111113
{
112-
symbol.BuildTypeId(Context, trapFile, symbol);
113-
trapFile.Write(";type");
114+
if (IsAnonymousType())
115+
trapFile.Write('*');
116+
else
117+
{
118+
symbol.BuildTypeId(Context, trapFile, symbol);
119+
trapFile.Write(";type");
120+
}
121+
}
122+
123+
public override void WriteQuotedId(TextWriter trapFile)
124+
{
125+
if (IsAnonymousType())
126+
trapFile.Write('*');
127+
else
128+
base.WriteQuotedId(trapFile);
114129
}
115130

116131
/// <summary>
@@ -167,22 +182,10 @@ public NamedTypeRef(Context cx, INamedTypeSymbol symbol) : base(cx, symbol)
167182
referencedType = Type.Create(cx, symbol);
168183
}
169184

170-
sealed class NamedTypeRefCacheKey
171-
{
172-
public readonly INamedTypeSymbol Symbol;
173-
public NamedTypeRefCacheKey(INamedTypeSymbol symbol) => Symbol = symbol;
174-
175-
public override int GetHashCode() =>
176-
11 * Symbol.GetHashCode();
177-
178-
public override bool Equals(object obj) =>
179-
obj is NamedTypeRefCacheKey k && SymbolEqualityComparer.IncludeNullability.Equals(k.Symbol, Symbol);
180-
}
181-
182185
public static NamedTypeRef Create(Context cx, INamedTypeSymbol type) =>
183186
// We need to use a different cache key than `type` to avoid mixing up
184187
// `NamedType`s and `NamedTypeRef`s
185-
NamedTypeRefFactory.Instance.CreateEntity(cx, new NamedTypeRefCacheKey(type), type);
188+
NamedTypeRefFactory.Instance.CreateEntity(cx, (typeof(NamedTypeRef), new SymbolEqualityWrapper(type)), type);
186189

187190
class NamedTypeRefFactory : ICachedEntityFactory<INamedTypeSymbol, NamedTypeRef>
188191
{

csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NullType.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ public override bool Equals(object obj)
2727
return obj != null && obj.GetType() == typeof(NullType);
2828
}
2929

30-
static readonly object cacheKey = new object();
31-
public static AnnotatedType Create(Context cx) => new AnnotatedType(NullTypeFactory.Instance.CreateEntity(cx, cacheKey, null), NullableAnnotation.None);
30+
public static AnnotatedType Create(Context cx) => new AnnotatedType(NullTypeFactory.Instance.CreateEntity(cx, typeof(NullType), null), NullableAnnotation.None);
3231

3332
class NullTypeFactory : ICachedEntityFactory<ITypeSymbol, NullType>
3433
{

csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -292,22 +292,10 @@ class DelegateTypeParameter : Parameter
292292
DelegateTypeParameter(Context cx, IParameterSymbol init, IEntity parent, Parameter original)
293293
: base(cx, init, parent, original) { }
294294

295-
sealed class DelegateTypeParameterCacheKey
296-
{
297-
public readonly IParameterSymbol Symbol;
298-
public DelegateTypeParameterCacheKey(IParameterSymbol symbol) => Symbol = symbol;
299-
300-
public override int GetHashCode() =>
301-
13 * Symbol.GetHashCode();
302-
303-
public override bool Equals(object obj) =>
304-
obj is DelegateTypeParameterCacheKey k && SymbolEqualityComparer.IncludeNullability.Equals(k.Symbol, Symbol);
305-
}
306-
307295
new public static DelegateTypeParameter Create(Context cx, IParameterSymbol param, IEntity parent, Parameter original = null) =>
308296
// We need to use a different cache key than `param` to avoid mixing up
309297
// `DelegateTypeParameter`s and `Parameter`s
310-
DelegateTypeParameterFactory.Instance.CreateEntity(cx, new DelegateTypeParameterCacheKey(param), (param, parent, original));
298+
DelegateTypeParameterFactory.Instance.CreateEntity(cx, (typeof(DelegateTypeParameter), new SymbolEqualityWrapper(param)), (param, parent, original));
311299

312300
class DelegateTypeParameterFactory : ICachedEntityFactory<(IParameterSymbol, IEntity, Parameter), DelegateTypeParameter>
313301
{

csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ static void BuildOrWriteId(this ISymbol symbol, Context cx, TextWriter trapFile,
212212
// #123 = @"C`1 : IEnumerable<__self___T>"
213213
// ```
214214
if (SymbolEqualityComparer.Default.Equals(symbol, symbolBeingDefined))
215-
trapFile.Write($"__self__");
215+
trapFile.Write("__self__");
216216
else if (symbol is ITypeSymbol type && type.IdDependsOn(cx, symbolBeingDefined))
217217
type.BuildTypeId(cx, trapFile, symbolBeingDefined, addBaseClass);
218218
else
@@ -294,12 +294,7 @@ void AddContaining()
294294
}
295295
}
296296

297-
if (named.IsAnonymousType)
298-
{
299-
AddContaining();
300-
named.BuildAnonymousName(cx, trapFile, (s0, cx0, trapFile0) => BuildOrWriteId(s0, cx0, trapFile0, symbolBeingDefined, addBaseClass), true);
301-
}
302-
else if (named.TypeParameters.IsEmpty)
297+
if (named.TypeParameters.IsEmpty)
303298
{
304299
AddContaining();
305300
trapFile.Write(named.Name);
@@ -310,20 +305,6 @@ void AddContaining()
310305
trapFile.Write(named.Name);
311306
trapFile.Write("`");
312307
trapFile.Write(named.TypeParameters.Length);
313-
// Some types such as `<>f__AnonymousType0` are not considered anonymous types by Roslyn,
314-
// perhaps because they contain type parameters. We still need to treat them like anonymous
315-
// types, though, by adding the underlying properties, in order to disambiguate them
316-
if (named.Name.Contains("__AnonymousType"))
317-
{
318-
trapFile.Write('<');
319-
trapFile.BuildList(",", named.GetMembers().OfType<IPropertySymbol>(), (prop, tb0) =>
320-
{
321-
tb0.Write(prop.Name);
322-
tb0.Write(" ");
323-
prop.Type.BuildOrWriteId(cx, tb0, symbolBeingDefined, addBaseClass);
324-
});
325-
trapFile.Write('>');
326-
}
327308
}
328309
else
329310
{
@@ -364,22 +345,14 @@ static void BuildNamespace(this INamespaceSymbol ns, Context cx, TextWriter trap
364345
trapFile.Write('.');
365346
}
366347

367-
static void BuildAnonymousName(this INamedTypeSymbol type, Context cx, TextWriter trapFile, Action<ITypeSymbol, Context, TextWriter> subTermAction, bool includeParamName)
348+
static void BuildAnonymousName(this INamedTypeSymbol type, Context cx, TextWriter trapFile)
368349
{
369-
var buildParam = includeParamName
370-
? (prop, tb0) =>
371-
{
372-
tb0.Write(prop.Name);
373-
tb0.Write(' ');
374-
subTermAction(prop.Type, cx, tb0);
375-
}
376-
: (Action<IPropertySymbol, TextWriter>)((prop, tb0) => subTermAction(prop.Type, cx, tb0));
377350
int memberCount = type.GetMembers().OfType<IPropertySymbol>().Count();
378351
int hackTypeNumber = memberCount == 1 ? 1 : 0;
379352
trapFile.Write("<>__AnonType");
380353
trapFile.Write(hackTypeNumber);
381354
trapFile.Write('<');
382-
trapFile.BuildList(",", type.GetMembers().OfType<IPropertySymbol>(), buildParam);
355+
trapFile.BuildList(",", type.GetMembers().OfType<IPropertySymbol>(), (prop, tb0) => BuildDisplayName(prop.Type, cx, tb0));
383356
trapFile.Write('>');
384357
}
385358

@@ -444,11 +417,9 @@ public static void BuildNamedTypeDisplayName(this INamedTypeSymbol namedType, Co
444417
}
445418

446419
if (namedType.IsAnonymousType)
447-
{
448-
namedType.BuildAnonymousName(cx, trapFile, (sub, cx0, tb0) => sub.BuildDisplayName(cx0, tb0), false);
449-
}
450-
451-
trapFile.Write(namedType.Name);
420+
namedType.BuildAnonymousName(cx, trapFile);
421+
else
422+
trapFile.Write(namedType.Name);
452423
if (namedType.IsGenericType && namedType.TypeKind != TypeKind.Error && namedType.TypeArguments.Any())
453424
{
454425
trapFile.Write('<');

csharp/extractor/Semmle.Extraction/Entities/File.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ public static string PathAsDatabaseId(string path)
8989

9090
public static string PathAsDatabaseString(string path) => path.Replace('\\', '/');
9191

92-
static readonly object cacheKey = new object();
93-
public static File Create(Context cx, string path) => FileFactory.Instance.CreateEntity(cx, (cacheKey, path), path);
92+
public static File Create(Context cx, string path) => FileFactory.Instance.CreateEntity(cx, (typeof(File), path), path);
9493

9594
public static File CreateGenerated(Context cx) => GeneratedFile.Create(cx);
9695

@@ -112,7 +111,7 @@ public override void WriteId(TextWriter trapFile)
112111
}
113112

114113
public static GeneratedFile Create(Context cx) =>
115-
GeneratedFileFactory.Instance.CreateEntity(cx, cacheKey, null);
114+
GeneratedFileFactory.Instance.CreateEntity(cx, typeof(GeneratedFile), null);
116115

117116
class GeneratedFileFactory : ICachedEntityFactory<string?, GeneratedFile>
118117
{

csharp/extractor/Semmle.Extraction/Entities/GeneratedLocation.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ public override void WriteId(TextWriter trapFile)
2828

2929
public override bool Equals(object? obj) => obj != null && obj.GetType() == typeof(GeneratedLocation);
3030

31-
static readonly object cacheKey = new object();
32-
public static GeneratedLocation Create(Context cx) => GeneratedLocationFactory.Instance.CreateEntity(cx, cacheKey, null);
31+
public static GeneratedLocation Create(Context cx) => GeneratedLocationFactory.Instance.CreateEntity(cx, typeof(GeneratedLocation), null);
3332

3433
class GeneratedLocationFactory : ICachedEntityFactory<string?, GeneratedLocation>
3534
{

csharp/extractor/Semmle.Extraction/Symbol.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.IO;
2+
using Microsoft.CodeAnalysis;
23

34
namespace Semmle.Extraction
45
{
@@ -75,4 +76,20 @@ public override bool Equals(object? obj)
7576

7677
public abstract TrapStackBehaviour TrapStackBehaviour { get; }
7778
}
79+
80+
/// <summary>
81+
/// A class used to wrap an `ISymbol` object, which uses `SymbolEqualityComparer.Default`
82+
/// for comparison.
83+
/// </summary>
84+
public sealed class SymbolEqualityWrapper
85+
{
86+
public ISymbol Symbol { get; }
87+
88+
public SymbolEqualityWrapper(ISymbol symbol) { Symbol = symbol; }
89+
90+
public override bool Equals(object? other) =>
91+
other is SymbolEqualityWrapper sew && SymbolEqualityComparer.Default.Equals(Symbol, sew.Symbol);
92+
93+
public override int GetHashCode() => 11 * Symbol.GetHashCode();
94+
}
7895
}

0 commit comments

Comments
 (0)