Skip to content

Commit 1bc65a6

Browse files
tamasvajktamasvajk
committed
Address PR review comments
1 parent a7451a1 commit 1bc65a6

File tree

12 files changed

+45
-60
lines changed

12 files changed

+45
-60
lines changed

‎csharp/extractor/Semmle.Extraction.CIL/Entities/ArrayType.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ public override bool Equals(object? obj)
3131

3232
public override void WriteId(TextWriter trapFile, bool inContext)
3333
{
34-
elementType.GetId(trapFile, inContext);
34+
elementType.WriteId(trapFile, inContext);
3535
trapFile.Write('[');
36-
if (rank > 1)
36+
for (var i = 1; i < rank; ++i)
3737
{
38-
trapFile.Write(new string(',', rank - 1));
38+
trapFile.Write(',');
3939
}
4040
trapFile.Write(']');
4141
}

‎csharp/extractor/Semmle.Extraction.CIL/Entities/Attribute.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public override IEnumerable<IExtractionProduct> Contents
4343
{
4444
decoded = attrib.DecodeValue(new CustomAttributeDecoder(Cx));
4545
}
46-
catch (Exception exc)
46+
catch
4747
{
4848
Cx.Cx.Extractor.Logger.Log(Util.Logging.Severity.Info,
4949
$"Attribute decoding is partial. Decoding attribute {constructor.DeclaringType.GetQualifiedName()} failed on {@object}.");
@@ -59,7 +59,7 @@ public override IEnumerable<IExtractionProduct> Contents
5959
foreach (var p in decoded.NamedArguments)
6060
{
6161
var stringValue = GetStringValue(p.Type, p.Value);
62-
yield return Tuples.cil_attribute_named_argument(this, p.Name, stringValue);
62+
yield return Tuples.cil_attribute_named_argument(this, p.Name!, stringValue);
6363
}
6464
}
6565
}

‎csharp/extractor/Semmle.Extraction.CIL/Entities/CustomAttributeDecoder.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,9 @@ public Type GetTypeFromReference(MetadataReader reader, TypeReferenceHandle hand
3030
public PrimitiveTypeCode GetUnderlyingEnumType(Type type)
3131
{
3232
if (type is TypeDefinitionType tdt &&
33-
tdt.GetUnderlyingEnumType() is var underlying &&
34-
underlying.HasValue)
33+
tdt.GetUnderlyingEnumType() is PrimitiveTypeCode underlying)
3534
{
36-
return underlying.Value;
35+
return underlying;
3736
}
3837

3938
var name = type.GetQualifiedName();

‎csharp/extractor/Semmle.Extraction.CIL/Entities/DefinitionMethod.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public override IEnumerable<IExtractionProduct> Contents
178178
}
179179
}
180180

181-
private IEnumerable<IExtractionProduct> Decode(byte[] ilbytes, Dictionary<int, Instruction> jump_table)
181+
private IEnumerable<IExtractionProduct> Decode(byte[]? ilbytes, Dictionary<int, Instruction> jump_table)
182182
{
183183
// Sequence points are stored in order of offset.
184184
// We use an enumerator to locate the correct sequence point for each instruction.
@@ -203,9 +203,9 @@ private IEnumerable<IExtractionProduct> Decode(byte[] ilbytes, Dictionary<int, I
203203
}
204204

205205
var child = 0;
206-
for (var offset = 0; offset < ilbytes.Length;)
206+
for (var offset = 0; offset < (ilbytes?.Length ?? 0);)
207207
{
208-
var instruction = new Instruction(Cx, this, ilbytes, offset, child++);
208+
var instruction = new Instruction(Cx, this, ilbytes!, offset, child++);
209209
yield return instruction;
210210

211211
if (nextSequencePoint != null && offset >= nextSequencePoint.Current.Offset)
@@ -245,12 +245,12 @@ public IEnumerable<Instruction> DebugInstructions
245245
var ilbytes = body.GetILBytes();
246246

247247
var child = 0;
248-
for (var offset = 0; offset < ilbytes.Length;)
248+
for (var offset = 0; offset < (ilbytes?.Length ?? 0);)
249249
{
250250
Instruction decoded;
251251
try
252252
{
253-
decoded = new Instruction(Cx, this, ilbytes, offset, child++);
253+
decoded = new Instruction(Cx, this, ilbytes!, offset, child++);
254254
offset += decoded.Width;
255255
}
256256
catch // lgtm[cs/catch-of-all-exceptions]

‎csharp/extractor/Semmle.Extraction.CIL/Entities/GenericsHelper.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ public static TypeTypeParameter[] MakeTypeParameters(Type container, int count)
1010
var newTypeParams = new TypeTypeParameter[count];
1111
for (var i = 0; i < newTypeParams.Length; ++i)
1212
{
13-
newTypeParams[i] = new TypeTypeParameter(container, container, i);
13+
newTypeParams[i] = new TypeTypeParameter(container, i);
1414
}
1515
return newTypeParams;
1616
}

‎csharp/extractor/Semmle.Extraction.CIL/Entities/NamedTypeIdWriter.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public void WriteId(TextWriter trapFile, bool inContext)
2424
var ct = type.ContainingType;
2525
if (ct != null)
2626
{
27-
ct.GetId(trapFile, inContext);
27+
ct.WriteId(trapFile, inContext);
2828
trapFile.Write('.');
2929
}
3030
else

‎csharp/extractor/Semmle.Extraction.CIL/Entities/NoMetadataHandleType.FullyQualifiedNameParser.cs‎

Lines changed: 23 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Linq;
4+
using Semmle.Util;
45

56
namespace Semmle.Extraction.CIL.Entities
67
{
@@ -33,7 +34,7 @@ public FullyQualifiedNameParser(string name)
3334
{
3435
ExtractAssemblyName(ref name, out var lastBracketIndex);
3536
ExtractTypeArguments(ref name, lastBracketIndex, out var containerTypeArguments);
36-
ExtractContainer(ref name, containerTypeArguments);
37+
ContainerName = ExtractContainer(ref name, containerTypeArguments);
3738

3839
ShortName = name;
3940
}
@@ -70,51 +71,43 @@ private void ExtractTypeArguments(ref string name, int lastBracketIndex, out str
7071

7172
TypeArguments = thisTypeArgs;
7273

73-
if (string.IsNullOrWhiteSpace(containerTypeArgs))
74-
{
75-
// containing type is not constructed generics
76-
containerTypeArguments = "";
77-
}
78-
else
79-
{
80-
// "T3,[T4, Assembly1, Version=...],,]"
81-
containerTypeArguments = $"[{containerTypeArgs}]";
82-
}
74+
containerTypeArguments = string.IsNullOrWhiteSpace(containerTypeArgs)
75+
? "" // containing type is not constructed generics
76+
: $"[{containerTypeArgs}]"; // "T3,[T4, Assembly1, Version=...],,]"
8377

8478
UnboundGenericTypeName = $"{name}{AssemblySuffix}";
8579
}
8680

87-
private void ExtractContainer(ref string name, string containerTypeArguments)
81+
private string ExtractContainer(ref string name, string containerTypeArguments)
8882
{
8983
var lastPlusIndex = name.LastIndexOf('+');
9084
IsContainerNamespace = lastPlusIndex < 0;
9185
if (IsContainerNamespace)
9286
{
93-
ExtractContainerNamespace(ref name);
94-
}
95-
else
96-
{
97-
ExtractContainerType(ref name, containerTypeArguments, lastPlusIndex);
87+
return ExtractContainerNamespace(ref name);
9888
}
89+
90+
return ExtractContainerType(ref name, containerTypeArguments, lastPlusIndex);
9991
}
10092

101-
private void ExtractContainerNamespace(ref string name)
93+
private static string ExtractContainerNamespace(ref string name)
10294
{
10395
var lastDotIndex = name.LastIndexOf('.');
10496
if (lastDotIndex >= 0)
10597
{
106-
(ContainerName, _, name) = name.Split(lastDotIndex, lastDotIndex + 1);
107-
}
108-
else
109-
{
110-
ContainerName = ""; // global namespace name
98+
string containerName;
99+
(containerName, _, name) = name.Split(lastDotIndex, lastDotIndex + 1);
100+
return containerName;
111101
}
102+
103+
return ""; // global namespace name
112104
}
113105

114-
private void ExtractContainerType(ref string name, string containerTypeArguments, int lastPlusIndex)
106+
private string ExtractContainerType(ref string name, string containerTypeArguments, int lastPlusIndex)
115107
{
116-
(ContainerName, _, name) = name.Split(lastPlusIndex, lastPlusIndex + 1);
117-
ContainerName = $"{ContainerName}{containerTypeArguments}{AssemblySuffix}";
108+
string containerName;
109+
(containerName, _, name) = name.Split(lastPlusIndex, lastPlusIndex + 1);
110+
return $"{containerName}{containerTypeArguments}{AssemblySuffix}";
118111
}
119112

120113
private void ExtractAssemblyName(ref string name, out int lastBracketIndex)
@@ -157,14 +150,10 @@ private static (string, IEnumerable<string>) ParseTypeArgumentStrings(string typ
157150
thisTypeArgs.Push(typeArgs[(startCurrentType + 1)..^1]);
158151
}
159152

160-
if (startCurrentType != 0)
161-
{
162-
typeArgs = typeArgs.Substring(0, startCurrentType - 1);
163-
}
164-
else
165-
{
166-
typeArgs = "";
167-
}
153+
typeArgs = startCurrentType != 0
154+
? typeArgs.Substring(0, startCurrentType - 1)
155+
: "";
156+
168157
thisTypeArgCount--;
169158
}
170159
return (typeArgs, thisTypeArgs.ToList());

‎csharp/extractor/Semmle.Extraction.CIL/Entities/SignatureDecoder.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ public void WriteId(TextWriter trapFile, GenericContext gc)
2121
{
2222
elementType.WriteId(trapFile, gc);
2323
trapFile.Write('[');
24-
if (shape.Rank > 1)
24+
for (var i = 1; i < shape.Rank; ++i)
2525
{
26-
trapFile.Write(string.Join(",", shape.Rank - 1));
26+
trapFile.Write(',');
2727
}
2828
trapFile.Write(']');
2929
}

‎csharp/extractor/Semmle.Extraction.CIL/Entities/Type.cs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@ protected Type(Context cx) : base(cx) { }
4848

4949
public sealed override void WriteId(TextWriter trapFile) => WriteId(trapFile, false);
5050

51-
public void GetId(TextWriter trapFile, bool inContext) => WriteId(trapFile, inContext);
52-
5351
/// <summary>
5452
/// Returns the friendly qualified name of types, such as
5553
/// ``"System.Collection.Generic.List`1"`` or
@@ -60,7 +58,7 @@ protected Type(Context cx) : base(cx) { }
6058
public string GetQualifiedName()
6159
{
6260
using var writer = new StringWriter();
63-
GetId(writer, false);
61+
WriteId(writer, false);
6462
var name = writer.ToString();
6563
return name.Substring(name.IndexOf(AssemblyTypeNameSeparator) + 2);
6664
}
@@ -107,9 +105,8 @@ public virtual IEnumerable<Type> ThisTypeArguments
107105
/// The total number of type parameters/type arguments (including parent types).
108106
/// This is used for internal consistency checking only.
109107
/// </summary>
110-
public int TotalTypeParametersCount => ContainingType == null
111-
? ThisTypeParameterCount
112-
: ThisTypeParameterCount + ContainingType.TotalTypeParametersCount;
108+
public int TotalTypeParametersCount =>
109+
ThisTypeParameterCount + (ContainingType?.TotalTypeParametersCount ?? 0);
113110

114111
/// <summary>
115112
/// Returns all bound/unbound generic arguments of a constructed/unbound generic type.

‎csharp/extractor/Semmle.Extraction.CIL/Entities/TypeDefinitionType.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ private IEnumerable<TypeTypeParameter> MakeTypeParameters()
8585

8686
// Two-phase population because type parameters can be mutually dependent
8787
for (var i = 0; i < newTypeParams.Length; ++i)
88-
newTypeParams[i] = Cx.Populate(new TypeTypeParameter(this, this, i));
88+
newTypeParams[i] = Cx.Populate(new TypeTypeParameter(this, i));
8989
for (var i = 0; i < newTypeParams.Length; ++i)
9090
newTypeParams[i].PopulateHandle(genericParams[i + toSkip]);
9191
return newTypeParams;

0 commit comments

Comments
 (0)