Skip to content

Commit 6cbff13

Browse files
authored
Merge pull request #4905 from tamasvajk/fix/attribute-argument-extraction
C#: Fix attribute argument extraction
2 parents 4100973 + e00db46 commit 6cbff13

File tree

13 files changed

+300
-213
lines changed

13 files changed

+300
-213
lines changed

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,25 @@ public override void Populate(TextWriter trapFile)
6666

6767
private void ExtractArguments(TextWriter trapFile)
6868
{
69+
var ctorArguments = attributeSyntax?.ArgumentList?.Arguments.Where(a => a.NameEquals == null).ToList();
70+
6971
var childIndex = 0;
70-
foreach (var constructorArgument in symbol.ConstructorArguments)
72+
for (var i = 0; i < symbol.ConstructorArguments.Length; i++)
7173
{
74+
var constructorArgument = symbol.ConstructorArguments[i];
75+
var paramName = symbol.AttributeConstructor?.Parameters[i].Name;
76+
var argSyntax = ctorArguments?.SingleOrDefault(a => a.NameColon != null && a.NameColon.Name.Identifier.Text == paramName);
77+
78+
if (argSyntax == null && // couldn't find named argument
79+
ctorArguments?.Count > childIndex && // there're more arguments
80+
ctorArguments[childIndex].NameColon == null) // the argument is positional
81+
{
82+
argSyntax = ctorArguments[childIndex];
83+
}
84+
7285
CreateExpressionFromArgument(
7386
constructorArgument,
74-
attributeSyntax?.ArgumentList.Arguments[childIndex].Expression,
87+
argSyntax?.Expression,
7588
this,
7689
childIndex++);
7790
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
using Microsoft.CodeAnalysis;
2+
3+
namespace Semmle.Extraction
4+
{
5+
/// <summary>
6+
/// The scope of symbols in an assembly.
7+
/// </summary>
8+
public class AssemblyScope : IExtractionScope
9+
{
10+
private readonly IAssemblySymbol assembly;
11+
private readonly string filepath;
12+
13+
public AssemblyScope(IAssemblySymbol symbol, string path, bool isOutput)
14+
{
15+
assembly = symbol;
16+
filepath = path;
17+
IsGlobalScope = isOutput;
18+
}
19+
20+
public bool IsGlobalScope { get; }
21+
22+
public bool InFileScope(string path) => path == filepath;
23+
24+
public bool InScope(ISymbol symbol) =>
25+
SymbolEqualityComparer.Default.Equals(symbol.ContainingAssembly, assembly) ||
26+
SymbolEqualityComparer.Default.Equals(symbol, assembly);
27+
28+
public bool FromSource => false;
29+
}
30+
}

csharp/extractor/Semmle.Extraction/Context.cs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ public void PopulateAll()
215215
}
216216
catch (Exception ex) // lgtm[cs/catch-of-all-exceptions]
217217
{
218-
ExtractionError("Uncaught exception", ex.Message, GeneratedLocation.Create(this), ex.StackTrace);
218+
ExtractionError("Uncaught exception", ex.Message, Entities.Location.Create(this), ex.StackTrace);
219219
}
220220
}
221221
}
@@ -250,6 +250,8 @@ public Context(IExtractor e, Compilation c, TrapWriter trapWriter, IExtractionSc
250250

251251
private IExtractionScope scope { get; }
252252

253+
public SyntaxTree? SourceTree => scope is SourceScope sc ? sc.SourceTree : null;
254+
253255
/// <summary>
254256
/// Whether the given symbol needs to be defined in this context.
255257
/// This is the case if the symbol is contained in the source/assembly, or
@@ -451,7 +453,7 @@ public void ExtractionError(string message, ISymbol? optionalSymbol, IEntity opt
451453
}
452454
else
453455
{
454-
ExtractionError(message, "", GeneratedLocation.Create(this));
456+
ExtractionError(message, "", Entities.Location.Create(this));
455457
}
456458
}
457459

@@ -522,13 +524,21 @@ public static void Try(this Context context, SyntaxNode? node, ISymbol? symbol,
522524
Message message;
523525

524526
if (node != null)
527+
{
525528
message = Message.Create(context, ex.Message, node, ex.StackTrace);
529+
}
526530
else if (symbol != null)
531+
{
527532
message = Message.Create(context, ex.Message, symbol, ex.StackTrace);
533+
}
528534
else if (ex is InternalError ie)
535+
{
529536
message = new Message(ie.Text, ie.EntityText, Entities.Location.Create(context, ie.Location), ex.StackTrace);
537+
}
530538
else
531-
message = new Message("Uncaught exception", ex.Message, GeneratedLocation.Create(context), ex.StackTrace);
539+
{
540+
message = new Message("Uncaught exception", ex.Message, Entities.Location.Create(context), ex.StackTrace);
541+
}
532542

533543
context.ExtractionError(message);
534544
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public ExtractionMessage(Context cx, Message msg) : base(cx)
1414

1515
protected override void Populate(TextWriter trapFile)
1616
{
17-
trapFile.extractor_messages(this, msg.Severity, "C# extractor", msg.Text, msg.EntityText, msg.Location ?? GeneratedLocation.Create(cx), msg.StackTrace);
17+
trapFile.extractor_messages(this, msg.Severity, "C# extractor", msg.Text, msg.EntityText, msg.Location ?? Location.Create(cx), msg.StackTrace);
1818
}
1919

2020
public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.NoLabel;

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

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

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

31-
public static GeneratedLocation Create(Context cx) => GeneratedLocationFactory.Instance.CreateEntity(cx, typeof(GeneratedLocation), null);
31+
public static new GeneratedLocation Create(Context cx) => GeneratedLocationFactory.Instance.CreateEntity(cx, typeof(GeneratedLocation), null);
3232

3333
private class GeneratedLocationFactory : ICachedEntityFactory<string?, GeneratedLocation>
3434
{

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11

2+
using Microsoft.CodeAnalysis.Text;
3+
24
namespace Semmle.Extraction.Entities
35
{
46
public abstract class Location : CachedEntity<Microsoft.CodeAnalysis.Location?>
@@ -13,6 +15,13 @@ public static Location Create(Context cx, Microsoft.CodeAnalysis.Location? loc)
1315
? NonGeneratedSourceLocation.Create(cx, loc)
1416
: Assembly.Create(cx, loc);
1517

18+
public static Location Create(Context cx)
19+
{
20+
return cx.SourceTree == null
21+
? GeneratedLocation.Create(cx)
22+
: Create(cx, Microsoft.CodeAnalysis.Location.Create(cx.SourceTree, TextSpan.FromBounds(0, 0)));
23+
}
24+
1625
public override Microsoft.CodeAnalysis.Location? ReportingLocation => symbol;
1726

1827
public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.OptionalLabel;

csharp/extractor/Semmle.Extraction/ExtractionScope.cs

Lines changed: 0 additions & 78 deletions
This file was deleted.
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
using Microsoft.CodeAnalysis;
2+
3+
namespace Semmle.Extraction
4+
{
5+
/// <summary>
6+
/// Defines which entities belong in the trap file
7+
/// for the currently extracted entity. This is used to ensure that
8+
/// trap files do not contain redundant information. Generally a symbol
9+
/// should have an affinity with exactly one trap file, except for constructed
10+
/// symbols.
11+
/// </summary>
12+
public interface IExtractionScope
13+
{
14+
/// <summary>
15+
/// Whether the given symbol belongs in the trap file.
16+
/// </summary>
17+
/// <param name="symbol">The symbol to populate.</param>
18+
bool InScope(ISymbol symbol);
19+
20+
/// <summary>
21+
/// Whether the given file belongs in the trap file.
22+
/// </summary>
23+
/// <param name="path">The path to populate.</param>
24+
bool InFileScope(string path);
25+
26+
bool IsGlobalScope { get; }
27+
28+
bool FromSource { get; }
29+
}
30+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
using Microsoft.CodeAnalysis;
2+
using System.Linq;
3+
4+
namespace Semmle.Extraction
5+
{
6+
7+
/// <summary>
8+
/// The scope of symbols in a source file.
9+
/// </summary>
10+
public class SourceScope : IExtractionScope
11+
{
12+
public SyntaxTree SourceTree { get; }
13+
14+
public SourceScope(SyntaxTree tree)
15+
{
16+
SourceTree = tree;
17+
}
18+
19+
public bool IsGlobalScope => false;
20+
21+
public bool InFileScope(string path) => path == SourceTree.FilePath;
22+
23+
public bool InScope(ISymbol symbol) => symbol.Locations.Any(loc => loc.SourceTree == SourceTree);
24+
25+
public bool FromSource => true;
26+
}
27+
}

csharp/ql/test/library-tests/attributes/AttributeArguments.expected

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,23 @@ arguments
4747
| attributes.cs:57:6:57:16 | [My(...)] | 0 | attributes.cs:57:18:57:21 | true |
4848
| attributes.cs:57:6:57:16 | [My(...)] | 1 | attributes.cs:57:28:57:29 | "" |
4949
| attributes.cs:57:6:57:16 | [My(...)] | 2 | attributes.cs:57:36:57:36 | 0 |
50-
| attributes.cs:76:2:76:5 | [Args(...)] | 0 | attributes.cs:76:7:76:8 | 42 |
51-
| attributes.cs:76:2:76:5 | [Args(...)] | 1 | attributes.cs:76:11:76:14 | null |
52-
| attributes.cs:76:2:76:5 | [Args(...)] | 2 | attributes.cs:76:17:76:25 | typeof(...) |
53-
| attributes.cs:76:2:76:5 | [Args(...)] | 3 | attributes.cs:76:28:76:30 | access to constant A |
54-
| attributes.cs:76:2:76:5 | [Args(...)] | 4 | attributes.cs:76:33:76:53 | array creation of type Int32[] |
55-
| attributes.cs:76:2:76:5 | [Args(...)] | 5 | attributes.cs:76:63:76:93 | array creation of type Object[] |
56-
| attributes.cs:79:6:79:9 | [Args(...)] | 0 | attributes.cs:79:11:79:16 | ... + ... |
57-
| attributes.cs:79:6:79:9 | [Args(...)] | 1 | attributes.cs:79:19:79:39 | array creation of type Int32[] |
58-
| attributes.cs:79:6:79:9 | [Args(...)] | 2 | attributes.cs:79:42:79:45 | null |
59-
| attributes.cs:79:6:79:9 | [Args(...)] | 3 | attributes.cs:79:48:79:52 | (...) ... |
60-
| attributes.cs:79:6:79:9 | [Args(...)] | 4 | attributes.cs:79:55:79:58 | null |
61-
| attributes.cs:79:6:79:9 | [Args(...)] | 5 | attributes.cs:79:68:79:98 | array creation of type Object[] |
50+
| attributes.cs:58:6:58:8 | [My2(...)] | 0 | attributes.cs:58:28:58:32 | false |
51+
| attributes.cs:58:6:58:8 | [My2(...)] | 1 | attributes.cs:58:13:58:16 | true |
52+
| attributes.cs:58:6:58:8 | [My2(...)] | 2 | attributes.cs:58:6:58:8 | 12 |
53+
| attributes.cs:58:6:58:8 | [My2(...)] | 3 | attributes.cs:58:22:58:22 | 1 |
54+
| attributes.cs:58:6:58:8 | [My2(...)] | 4 | attributes.cs:58:39:58:40 | 42 |
55+
| attributes.cs:77:2:77:5 | [Args(...)] | 0 | attributes.cs:77:7:77:8 | 42 |
56+
| attributes.cs:77:2:77:5 | [Args(...)] | 1 | attributes.cs:77:11:77:14 | null |
57+
| attributes.cs:77:2:77:5 | [Args(...)] | 2 | attributes.cs:77:17:77:25 | typeof(...) |
58+
| attributes.cs:77:2:77:5 | [Args(...)] | 3 | attributes.cs:77:28:77:30 | access to constant A |
59+
| attributes.cs:77:2:77:5 | [Args(...)] | 4 | attributes.cs:77:33:77:53 | array creation of type Int32[] |
60+
| attributes.cs:77:2:77:5 | [Args(...)] | 5 | attributes.cs:77:63:77:93 | array creation of type Object[] |
61+
| attributes.cs:80:6:80:9 | [Args(...)] | 0 | attributes.cs:80:11:80:16 | ... + ... |
62+
| attributes.cs:80:6:80:9 | [Args(...)] | 1 | attributes.cs:80:19:80:39 | array creation of type Int32[] |
63+
| attributes.cs:80:6:80:9 | [Args(...)] | 2 | attributes.cs:80:42:80:45 | null |
64+
| attributes.cs:80:6:80:9 | [Args(...)] | 3 | attributes.cs:80:48:80:52 | (...) ... |
65+
| attributes.cs:80:6:80:9 | [Args(...)] | 4 | attributes.cs:80:55:80:58 | null |
66+
| attributes.cs:80:6:80:9 | [Args(...)] | 5 | attributes.cs:80:68:80:98 | array creation of type Object[] |
6267
constructorArguments
6368
| Assembly1.dll:0:0:0:0 | [Custom(...)] | 0 | Assembly1.dll:0:0:0:0 | 1 |
6469
| Assembly1.dll:0:0:0:0 | [Custom(...)] | 0 | Assembly1.dll:0:0:0:0 | 3 |
@@ -101,16 +106,20 @@ constructorArguments
101106
| attributes.cs:46:6:46:16 | [Conditional(...)] | 0 | attributes.cs:46:18:46:25 | "DEBUG2" |
102107
| attributes.cs:54:6:54:16 | [My(...)] | 0 | attributes.cs:54:18:54:22 | false |
103108
| attributes.cs:57:6:57:16 | [My(...)] | 0 | attributes.cs:57:18:57:21 | true |
104-
| attributes.cs:76:2:76:5 | [Args(...)] | 0 | attributes.cs:76:7:76:8 | 42 |
105-
| attributes.cs:76:2:76:5 | [Args(...)] | 1 | attributes.cs:76:11:76:14 | null |
106-
| attributes.cs:76:2:76:5 | [Args(...)] | 2 | attributes.cs:76:17:76:25 | typeof(...) |
107-
| attributes.cs:76:2:76:5 | [Args(...)] | 3 | attributes.cs:76:28:76:30 | access to constant A |
108-
| attributes.cs:76:2:76:5 | [Args(...)] | 4 | attributes.cs:76:33:76:53 | array creation of type Int32[] |
109-
| attributes.cs:79:6:79:9 | [Args(...)] | 0 | attributes.cs:79:11:79:16 | ... + ... |
110-
| attributes.cs:79:6:79:9 | [Args(...)] | 1 | attributes.cs:79:19:79:39 | array creation of type Int32[] |
111-
| attributes.cs:79:6:79:9 | [Args(...)] | 2 | attributes.cs:79:42:79:45 | null |
112-
| attributes.cs:79:6:79:9 | [Args(...)] | 3 | attributes.cs:79:48:79:52 | (...) ... |
113-
| attributes.cs:79:6:79:9 | [Args(...)] | 4 | attributes.cs:79:55:79:58 | null |
109+
| attributes.cs:58:6:58:8 | [My2(...)] | 0 | attributes.cs:58:28:58:32 | false |
110+
| attributes.cs:58:6:58:8 | [My2(...)] | 1 | attributes.cs:58:13:58:16 | true |
111+
| attributes.cs:58:6:58:8 | [My2(...)] | 2 | attributes.cs:58:6:58:8 | 12 |
112+
| attributes.cs:58:6:58:8 | [My2(...)] | 3 | attributes.cs:58:22:58:22 | 1 |
113+
| attributes.cs:77:2:77:5 | [Args(...)] | 0 | attributes.cs:77:7:77:8 | 42 |
114+
| attributes.cs:77:2:77:5 | [Args(...)] | 1 | attributes.cs:77:11:77:14 | null |
115+
| attributes.cs:77:2:77:5 | [Args(...)] | 2 | attributes.cs:77:17:77:25 | typeof(...) |
116+
| attributes.cs:77:2:77:5 | [Args(...)] | 3 | attributes.cs:77:28:77:30 | access to constant A |
117+
| attributes.cs:77:2:77:5 | [Args(...)] | 4 | attributes.cs:77:33:77:53 | array creation of type Int32[] |
118+
| attributes.cs:80:6:80:9 | [Args(...)] | 0 | attributes.cs:80:11:80:16 | ... + ... |
119+
| attributes.cs:80:6:80:9 | [Args(...)] | 1 | attributes.cs:80:19:80:39 | array creation of type Int32[] |
120+
| attributes.cs:80:6:80:9 | [Args(...)] | 2 | attributes.cs:80:42:80:45 | null |
121+
| attributes.cs:80:6:80:9 | [Args(...)] | 3 | attributes.cs:80:48:80:52 | (...) ... |
122+
| attributes.cs:80:6:80:9 | [Args(...)] | 4 | attributes.cs:80:55:80:58 | null |
114123
namedArguments
115124
| Assembly1.dll:0:0:0:0 | [Custom(...)] | Prop2 | Assembly1.dll:0:0:0:0 | array creation of type Object[] |
116125
| Assembly1.dll:0:0:0:0 | [Custom(...)] | Prop2 | Assembly1.dll:0:0:0:0 | array creation of type Object[] |
@@ -119,5 +128,6 @@ namedArguments
119128
| attributes.cs:41:10:41:13 | [Args(...)] | Prop | attributes.cs:41:90:41:120 | array creation of type Object[] |
120129
| attributes.cs:57:6:57:16 | [My(...)] | x | attributes.cs:57:36:57:36 | 0 |
121130
| attributes.cs:57:6:57:16 | [My(...)] | y | attributes.cs:57:28:57:29 | "" |
122-
| attributes.cs:76:2:76:5 | [Args(...)] | Prop | attributes.cs:76:63:76:93 | array creation of type Object[] |
123-
| attributes.cs:79:6:79:9 | [Args(...)] | Prop | attributes.cs:79:68:79:98 | array creation of type Object[] |
131+
| attributes.cs:58:6:58:8 | [My2(...)] | X | attributes.cs:58:39:58:40 | 42 |
132+
| attributes.cs:77:2:77:5 | [Args(...)] | Prop | attributes.cs:77:63:77:93 | array creation of type Object[] |
133+
| attributes.cs:80:6:80:9 | [Args(...)] | Prop | attributes.cs:80:68:80:98 | array creation of type Object[] |

0 commit comments

Comments
 (0)