Skip to content

Commit 546d750

Browse files
hvitvedhvitved
committed
C#: Reintroduce getURL()
It turns out that we still need `getURL()` to account for cases where there is no `getLocation()`. Not having `getURL()` for entities without a `getLocation()` results in a `file://0:0:0:0` URL, which is not rendered in QL4E, unlike a `""` URL.
1 parent ada0115 commit 546d750

File tree

18 files changed

+142
-111
lines changed

18 files changed

+142
-111
lines changed

‎csharp/ql/src/Useless code/DefaultToString.ql‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,34 @@ predicate alwaysDefaultToString(ValueOrRefType t) {
5858
)
5959
}
6060

61-
from Expr e, ValueOrRefType t
62-
where invokesToString(e, t)
63-
and alwaysDefaultToString(t)
64-
select e, "Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing.", t, t.getName()
61+
newtype TDefaultToStringType =
62+
TDefaultToStringType0(ValueOrRefType t) { alwaysDefaultToString(t) }
63+
64+
class DefaultToStringType extends TDefaultToStringType {
65+
ValueOrRefType t;
66+
67+
DefaultToStringType() { this = TDefaultToStringType0(t) }
68+
69+
ValueOrRefType getType() { result = t }
70+
71+
string toString() { result = t.toString() }
72+
73+
// A workaround for generating empty URLs for non-source locations, because qltest
74+
// does not support non-source locations
75+
string getURL() {
76+
exists(Location l |
77+
l = t.getLocation() |
78+
if l instanceof SourceLocation then
79+
exists(string path, int a, int b, int c, int d |
80+
l.hasLocationInfo(path, a, b, c, d) |
81+
toUrl(path, a, b, c, d, result)
82+
)
83+
else
84+
result = ""
85+
)
86+
}
87+
}
88+
89+
from Expr e, DefaultToStringType t
90+
where invokesToString(e, t.getType())
91+
select e, "Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing.", t, t.getType().getName()

‎csharp/ql/src/semmle/code/csharp/Element.qll‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ class Element extends DotNet::Element, @element {
2222
* Where an element has multiple locations (for example a source file and an assembly),
2323
* gets only the source location.
2424
*/
25-
override Location getLocation() { result = bestLocation(this) }
25+
override Location getLocation() { result = Internal::bestLocation(this) }
26+
27+
/** Gets the URL of this element. */
28+
string getURL() { result = Internal::getURL(this) }
2629

2730
/** Gets a location of this element, including sources and assemblies. */
2831
override Location getALocation() { none() }

‎csharp/ql/src/semmle/code/csharp/Enclosing.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*/
1414

1515
import Stmt
16-
private import semmle.code.csharp.ExprOrStmtParent
16+
private import semmle.code.csharp.ExprOrStmtParent as ExprOrStmtParent
1717

1818
/**
1919
* INTERNAL: Do not use.
@@ -35,7 +35,7 @@ cached module Internal {
3535
c.getAChildStmt+() = s
3636
}
3737

38-
private Expr getAChildExpr(ExprOrStmtParent p) {
38+
private Expr getAChildExpr(ExprOrStmtParent::ExprOrStmtParent p) {
3939
result = p.getAChildExpr() or
4040
result = p.(AssignOperation).getExpandedAssignment()
4141
}

‎csharp/ql/src/semmle/code/csharp/ExprOrStmtParent.qll‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ private int getImplementationSize(ValueOrRefType t, File f) {
346346
result = getImplementationSize1(t, f)
347347
}
348348

349-
private cached module Cached {
349+
cached module Internal {
350350
cached
351351
ControlFlowElement getTopLevelChild(ExprOrStmtParent p, int i) {
352352
result = p.(MultiImplementationsParent).getBestChild(i)
@@ -363,7 +363,7 @@ private cached module Cached {
363363
* in `f`.
364364
*/
365365
cached
366-
predicate mustHaveLocationInFileCached(Declaration d, File f) {
366+
predicate mustHaveLocationInFile(Declaration d, File f) {
367367
exists(MultiImplementationsParent p, ValueOrRefType t |
368368
t = getTopLevelDeclaringType(p) and
369369
f = p.getBestFile() |
@@ -381,7 +381,7 @@ private cached module Cached {
381381
* locations, choose only one.
382382
*/
383383
cached
384-
Location bestLocationCached(Element e) {
384+
Location bestLocation(Element e) {
385385
result = e.getALocation().(SourceLocation) and
386386
(mustHaveLocationInFile(e, _) implies mustHaveLocationInFile(e, result.getFile()))
387387
or
@@ -391,8 +391,17 @@ private cached module Cached {
391391
result = min(Location l | l = e.getALocation() | l order by l.getFile().toString())
392392
)
393393
}
394-
}
395-
private import Cached
396394

397-
predicate mustHaveLocationInFile = mustHaveLocationInFileCached/2;
398-
predicate bestLocation = bestLocationCached/1;
395+
cached
396+
string getURL(Element e) {
397+
exists(Location l, string path, int a, int b, int c, int d |
398+
l = bestLocation(e) |
399+
l.hasLocationInfo(path, a, b, c, d) and
400+
toUrl(path, a, b, c, d, result)
401+
)
402+
or
403+
not exists(bestLocation(e)) and
404+
result = ""
405+
}
406+
}
407+
private import Internal

‎csharp/ql/src/semmle/code/csharp/Type.qll‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -808,8 +808,7 @@ class DelegateType extends RefType, Parameterizable, @delegate_type {
808808
/**
809809
* The `null` type. The type of the `null` literal.
810810
*/
811-
class NullType extends RefType, @null_type {
812-
}
811+
class NullType extends RefType, @null_type { }
813812

814813
/**
815814
* A nullable type, for example `int?`.
@@ -890,6 +889,13 @@ class ArrayType extends DotNet::ArrayType, RefType, @array_type {
890889
}
891890

892891
override Type getChild(int n) { result = getElementType() and n = 0 }
892+
893+
override Location getALocation() {
894+
type_location(this, result)
895+
or
896+
not type_location(this, _) and
897+
result = this.getElementType().getALocation()
898+
}
893899
}
894900

895901
/**

‎csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowElement.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/** Provides the class `ControlFlowElement`. */
22

33
import csharp
4-
private import semmle.code.csharp.ExprOrStmtParent
4+
private import semmle.code.csharp.ExprOrStmtParent as ExprOrStmtParent
55
private import ControlFlow
66
private import ControlFlow::BasicBlocks
77
private import SuccessorTypes
@@ -15,7 +15,7 @@ private import SuccessorTypes
1515
* control flow elements and control flow nodes. This allows control flow
1616
* splitting, for example modeling the control flow through `finally` blocks.
1717
*/
18-
class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
18+
class ControlFlowElement extends ExprOrStmtParent::ExprOrStmtParent, @control_flow_element {
1919
/** Gets the enclosing callable of this element, if any. */
2020
Callable getEnclosingCallable() { none() }
2121

‎csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowGraph.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,7 +1769,7 @@ module ControlFlow {
17691769
* not return.
17701770
*/
17711771
private module NonReturning {
1772-
private import semmle.code.csharp.ExprOrStmtParent
1772+
private import semmle.code.csharp.ExprOrStmtParent as ExprOrStmtParent
17731773
private import semmle.code.csharp.commons.Assertions
17741774
private import semmle.code.csharp.frameworks.System
17751775

@@ -1810,7 +1810,7 @@ module ControlFlow {
18101810
private abstract class NonReturningCallable extends Callable {
18111811
NonReturningCallable() {
18121812
not exists(ReturnStmt ret | ret.getEnclosingCallable() = this) and
1813-
not hasAccessorAutoImplementation(this, _) and
1813+
not ExprOrStmtParent::hasAccessorAutoImplementation(this, _) and
18141814
not exists(Virtualizable v |
18151815
v.isOverridableOrImplementable() |
18161816
v = this or

‎csharp/ql/src/semmle/code/csharp/frameworks/Format.qll‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -159,17 +159,11 @@ class InvalidFormatString extends StringLiteral {
159159
result = this.getValue().regexpFind(getValidFormatRegex(),0,0).length()
160160
}
161161

162-
/**
163-
* Holds if this element is at the specified location.
164-
* The location spans column `startcolumn` of line `startline` to
165-
* column `endcolumn` of line `endline` in file `filepath`.
166-
* For more information, see
167-
* [LGTM locations](https://lgtm.com/help/ql/locations).
168-
*/
169-
predicate hasLocationInfo(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
170-
exists(int oldstartcolumn, int padding |
162+
override string getURL() {
163+
exists(string filepath, int startline, int startcolumn, int endline, int endcolumn, int oldstartcolumn, int padding |
171164
this.getLocation().hasLocationInfo(filepath, startline, oldstartcolumn, endline, endcolumn) and
172-
startcolumn = padding + oldstartcolumn + getInvalidOffset() |
165+
startcolumn = padding + oldstartcolumn + getInvalidOffset() and
166+
toUrl(filepath, startline, startcolumn, endline, endcolumn, result) |
173167
// Single-line string literal beginning " or @"
174168
// Figure out the correct indent.
175169
startline = endline and

‎csharp/ql/test/library-tests/async/Async.expected‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
| async.cs:11:29:11:32 | Main | file://:0:0:0:0 | private |
2-
| async.cs:11:29:11:32 | Main | file://:0:0:0:0 | static |
3-
| async.cs:22:35:22:57 | PrintContentLengthAsync | file://:0:0:0:0 | async |
4-
| async.cs:22:35:22:57 | PrintContentLengthAsync | file://:0:0:0:0 | private |
5-
| async.cs:22:35:22:57 | PrintContentLengthAsync | file://:0:0:0:0 | static |
6-
| async.cs:28:40:28:57 | ContentLengthAsync | file://:0:0:0:0 | async |
7-
| async.cs:28:40:28:57 | ContentLengthAsync | file://:0:0:0:0 | private |
8-
| async.cs:28:40:28:57 | ContentLengthAsync | file://:0:0:0:0 | static |
9-
| async.cs:38:35:38:47 | AsyncIterator | file://:0:0:0:0 | async |
10-
| async.cs:38:35:38:47 | AsyncIterator | file://:0:0:0:0 | private |
11-
| async.cs:38:35:38:47 | AsyncIterator | file://:0:0:0:0 | static |
12-
| async.cs:50:49:50:57 | OpenAsync | file://:0:0:0:0 | async |
13-
| async.cs:50:49:50:57 | OpenAsync | file://:0:0:0:0 | private |
14-
| async.cs:50:49:50:57 | OpenAsync | file://:0:0:0:0 | static |
1+
| async.cs:11:29:11:32 | Main | | private |
2+
| async.cs:11:29:11:32 | Main | | static |
3+
| async.cs:22:35:22:57 | PrintContentLengthAsync | | async |
4+
| async.cs:22:35:22:57 | PrintContentLengthAsync | | private |
5+
| async.cs:22:35:22:57 | PrintContentLengthAsync | | static |
6+
| async.cs:28:40:28:57 | ContentLengthAsync | | async |
7+
| async.cs:28:40:28:57 | ContentLengthAsync | | private |
8+
| async.cs:28:40:28:57 | ContentLengthAsync | | static |
9+
| async.cs:38:35:38:47 | AsyncIterator | | async |
10+
| async.cs:38:35:38:47 | AsyncIterator | | private |
11+
| async.cs:38:35:38:47 | AsyncIterator | | static |
12+
| async.cs:50:49:50:57 | OpenAsync | | async |
13+
| async.cs:50:49:50:57 | OpenAsync | | private |
14+
| async.cs:50:49:50:57 | OpenAsync | | static |

‎csharp/ql/test/library-tests/expressions/AnonymousObjectCreation3.expected‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
| expressions.cs:417:9:420:9 | object creation of type <>__AnonType0<String,String[]> | file://:0:0:0:0 | String[] |
2-
| expressions.cs:421:9:424:9 | object creation of type <>__AnonType0<String,String[]> | file://:0:0:0:0 | String[] |
1+
| expressions.cs:417:9:420:9 | object creation of type <>__AnonType0<String,String[]> | String[] |
2+
| expressions.cs:421:9:424:9 | object creation of type <>__AnonType0<String,String[]> | String[] |

0 commit comments

Comments
 (0)