Skip to content

Commit 7dd263b

Browse files
calumgrantcalumgrant
authored
Merge pull request #689 from hvitved/csharp/remove-get-url
C#: Remove `getUrl()` predicates
2 parents 1710f8d + c66f67d commit 7dd263b

25 files changed

+148
-133
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/cil/Type.qll‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ class Namespace extends DotNet::Namespace, TypeContainer, @namespace {
2323
override Namespace getParent() { result = this.getParentNamespace() }
2424
override Namespace getParentNamespace() { parent_namespace(this, result) }
2525
override string getName() { namespaces(this,result) }
26-
string getUrl() { result="" }
2726
override Location getLocation() { none() }
2827
}
2928

‎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 = ExprOrStmtParentCached::bestLocation(this) }
26+
27+
/** Gets the URL of this element. */
28+
string getURL() { result = ExprOrStmtParentCached::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/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 ExprOrStmtParentCached {
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 ExprOrStmtParentCached

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

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,16 @@ class Location extends @location
3737
string toString() { none() }
3838

3939
/** Gets the start line of this location. */
40-
int getStartLine() {
41-
locations_default(this,_,result,_,_,_)
42-
}
40+
final int getStartLine() { this.hasLocationInfo(_, result, _, _, _) }
4341

4442
/** Gets the end line of this location. */
45-
int getEndLine() {
46-
locations_default(this,_,_,_,result,_)
47-
}
43+
final int getEndLine() { this.hasLocationInfo(_, _, _, result, _) }
4844

4945
/** Gets the start column of this location. */
50-
int getStartColumn() {
51-
locations_default(this,_,_,result,_,_)
52-
}
46+
final int getStartColumn() { this.hasLocationInfo(_, _, result, _, _) }
5347

5448
/** Gets the end column of this location. */
55-
int getEndColumn() {
56-
locations_default(this,_,_,_,_,result)
57-
}
49+
final int getEndColumn() { this.hasLocationInfo(_, _, _, _, result) }
5850
}
5951

6052
/**

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,6 @@ class Declaration extends DotNet::Declaration, Element, @declaration {
3636
result = this.getDeclaringType().getQualifiedName() + "." + this.toStringWithTypes()
3737
}
3838

39-
/** Define URL for declarations with no location. */
40-
string getURL() {
41-
if exists(this.getLocation()) then
42-
exists(string path, int a, int b, int c, int d |
43-
this.getLocation().hasLocationInfo(path, a, b, c, d) |
44-
toUrl(path, a, b, c, d, result)
45-
)
46-
else
47-
result = ""
48-
}
49-
5039
/**
5140
* Holds if this declaration has been generated by the compiler, for example
5241
* implicit constructors or accessors.

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,12 @@ class Namespace extends DotNet::Namespace, TypeContainer, @namespace {
112112

113113
override predicate fromLibrary() { not this.fromSource() }
114114

115-
/** Gets the URL of this namespace (which is empty in this case). */
116-
string getURL() { result = "" }
115+
/** Gets a declaration of this namespace, if any. */
116+
NamespaceDeclaration getADeclaration() { result.getNamespace() = this }
117+
118+
override Location getALocation() {
119+
result = this.getADeclaration().getALocation()
120+
}
117121
}
118122

119123
/**
@@ -193,7 +197,7 @@ class NamespaceDeclaration extends Element, @namespace_declaration {
193197
*/
194198
ValueOrRefType getATypeDeclaration() { parent_namespace_declaration(result, this) }
195199

196-
override Location getALocation() { none() }
200+
override Location getALocation() { namespace_declaration_location(this, result) }
197201

198202
override string toString() { result = "namespace ... { ... }" }
199203
}

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -729,7 +729,6 @@ class Class extends RefType, @class_type {
729729
* ```
730730
*/
731731
class AnonymousClass extends Class {
732-
733732
AnonymousClass() { this.getName().matches("<%") }
734733
}
735734

@@ -787,8 +786,7 @@ class DelegateType extends RefType, Parameterizable, @delegate_type {
787786
/**
788787
* The `null` type. The type of the `null` literal.
789788
*/
790-
class NullType extends RefType, @null_type {
791-
}
789+
class NullType extends RefType, @null_type { }
792790

793791
/**
794792
* A nullable type, for example `int?`.
@@ -869,6 +867,13 @@ class ArrayType extends DotNet::ArrayType, RefType, @array_type {
869867
}
870868

871869
override Type getChild(int n) { result = getElementType() and n = 0 }
870+
871+
override Location getALocation() {
872+
type_location(this, result)
873+
or
874+
not type_location(this, _) and
875+
result = this.getElementType().getALocation()
876+
}
872877
}
873878

874879
/**

‎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 |

0 commit comments

Comments
 (0)