Skip to content

Commit 2a972dc

Browse files
hvitvedhvitved
committed
Address review comments
1 parent 85e1cda commit 2a972dc

File tree

5 files changed

+29
-21
lines changed

5 files changed

+29
-21
lines changed

‎ruby/ql/lib/codeql/ruby/ast/Literal.qll

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ private int parseInteger(Ruby::Integer i) {
7777
v = values.indexOf(c.toLowerCase()) and
7878
exp = str.replaceAll("_", "").length() - index - 1
7979
|
80-
v * values.length().pow(exp)
80+
v * values.length().pow(exp).floor()
8181
)
8282
)
8383
}
8484

85-
private class RequiredIntegerConstantValue extends RequiredConstantValue {
85+
private class RequiredIntegerLiteralConstantValue extends RequiredConstantValue {
8686
override predicate requiredInt(int i) { i = any(IntegerLiteral il).getValue() }
8787
}
8888

@@ -277,7 +277,7 @@ private class FalseLiteral extends BooleanLiteral, TFalseLiteral {
277277
final override predicate isFalse() { any() }
278278
}
279279

280-
private class RequiredStringConstantValue extends RequiredConstantValue {
280+
private class RequiredEncodingLiteralConstantValue extends RequiredConstantValue {
281281
override predicate requiredString(string s) { s = "UTF-8" }
282282
}
283283

@@ -293,7 +293,7 @@ class EncodingLiteral extends Literal, TEncoding {
293293
override ConstantValue::ConstantStringValue getConstantValue() { result.isString("UTF-8") }
294294
}
295295

296-
private class RequiredIntegerConstantValue2 extends RequiredConstantValue {
296+
private class RequiredLineLiteralConstantValue extends RequiredConstantValue {
297297
override predicate requiredInt(int i) { i = any(LineLiteral ll).getLocation().getStartLine() }
298298
}
299299

@@ -310,7 +310,7 @@ class LineLiteral extends Literal, TLine {
310310
}
311311
}
312312

313-
private class RequiredStringConstantValue2 extends RequiredConstantValue {
313+
private class RequiredFileLiteralConstantValue extends RequiredConstantValue {
314314
override predicate requiredString(string s) {
315315
s = any(FileLiteral fl).getLocation().getFile().getAbsolutePath()
316316
}
@@ -346,7 +346,7 @@ class StringComponent extends AstNode, TStringComponent {
346346
ConstantValue::ConstantStringValue getConstantValue() { none() }
347347
}
348348

349-
private class RequiredStringConstantValue3 extends RequiredConstantValue {
349+
private class RequiredStringTextComponentConstantValue extends RequiredConstantValue {
350350
override predicate requiredString(string s) {
351351
s = any(Ruby::Token t | exists(TStringTextComponentNonRegexp(t))).getValue()
352352
}
@@ -378,7 +378,7 @@ class StringTextComponent extends StringComponent, TStringTextComponentNonRegexp
378378
final override string getAPrimaryQlClass() { result = "StringTextComponent" }
379379
}
380380

381-
private class RequiredStringConstantValue4 extends RequiredConstantValue {
381+
private class RequiredStringEscapeSequenceComponentConstantValue extends RequiredConstantValue {
382382
override predicate requiredString(string s) {
383383
s = any(Ruby::Token t | exists(TStringEscapeSequenceComponentNonRegexp(t))).getValue()
384384
}
@@ -450,7 +450,7 @@ private string getRegExpTextComponentValue(RegExpTextComponent c) {
450450
)
451451
}
452452

453-
private class RequiredStringConstantValue5 extends RequiredConstantValue {
453+
private class RequiredRegExpTextComponentConstantValue extends RequiredConstantValue {
454454
override predicate requiredString(string s) { s = getRegExpTextComponentValue(_) }
455455
}
456456

@@ -490,7 +490,7 @@ private string getRegExpEscapeSequenceComponentValue(RegExpEscapeSequenceCompone
490490
)
491491
}
492492

493-
private class RequiredStringConstantValue6 extends RequiredConstantValue {
493+
private class RequiredRegExpEscapeSequenceComponentConstantValue extends RequiredConstantValue {
494494
override predicate requiredString(string s) { s = getRegExpEscapeSequenceComponentValue(_) }
495495
}
496496

@@ -761,7 +761,7 @@ class SymbolLiteral extends StringlikeLiteral, TSymbolLiteral {
761761
// Tree-sitter gives us value text including the colon, which we skip.
762762
private string getSimpleSymbolValue(Ruby::SimpleSymbol ss) { result = ss.getValue().suffix(1) }
763763

764-
private class RequiredSymbolConstantValue extends RequiredConstantValue {
764+
private class RequiredSimpleSymbolConstantValue extends RequiredConstantValue {
765765
override predicate requiredSymbol(string s) { s = getSimpleSymbolValue(_) }
766766
}
767767

@@ -795,7 +795,7 @@ private class BareSymbolLiteral extends ComplexSymbolLiteral, TBareSymbolLiteral
795795
final override StringComponent getComponent(int i) { toGenerated(result) = g.getChild(i) }
796796
}
797797

798-
private class RequiredSymbolConstantValue2 extends RequiredConstantValue {
798+
private class RequiredHashKeySymbolConstantValue extends RequiredConstantValue {
799799
override predicate requiredSymbol(string s) { s = any(Ruby::HashKeySymbol h).getValue() }
800800
}
801801

@@ -829,7 +829,7 @@ class SubshellLiteral extends StringlikeLiteral, TSubshellLiteral {
829829
final override StringComponent getComponent(int i) { toGenerated(result) = g.getChild(i) }
830830
}
831831

832-
private class RequiredStringConstantValue7 extends RequiredConstantValue {
832+
private class RequiredCharacterConstantValue extends RequiredConstantValue {
833833
override predicate requiredString(string s) { s = any(Ruby::Character c).getValue() }
834834
}
835835

@@ -1131,7 +1131,7 @@ private string getMethodName(MethodName::Token t) {
11311131
result = t.(Ruby::Setter).getName().getValue() + "="
11321132
}
11331133

1134-
private class RequiredStringConstantValue8 extends RequiredConstantValue {
1134+
private class RequiredMethodNameConstantValue extends RequiredConstantValue {
11351135
override predicate requiredString(string s) { s = getMethodName(_) }
11361136
}
11371137

‎ruby/ql/lib/codeql/ruby/ast/internal/Constant.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ newtype TConstantValue =
1515

1616
private newtype TRequiredConstantValue = MkRequiredConstantValue()
1717

18+
/**
19+
* A class that exists for QL technical reasons only (the IPA type used
20+
* to represent constant values needs to be bounded).
21+
*/
1822
class RequiredConstantValue extends MkRequiredConstantValue {
1923
string toString() { none() }
2024

‎ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ module ExprNodes {
272272
final ExprCfgNode getOperand() { e.hasCfgChild(uo.getOperand(), this, result) }
273273

274274
final override ConstantValue getConstantValue() {
275+
// TODO: Implement support for complex numbers and rational numbers
275276
exists(string op, ConstantValue value | unaryConstFold(this, op, value) |
276277
op = "+" and
277278
result = value
@@ -373,6 +374,7 @@ module ExprNodes {
373374
final ExprCfgNode getRightOperand() { e.hasCfgChild(bo.getRightOperand(), this, result) }
374375

375376
final override ConstantValue getConstantValue() {
377+
// TODO: Implement support for complex numbers and rational numbers
376378
exists(string op, ConstantValue left, ConstantValue right |
377379
binaryConstFold(this, op, left, right)
378380
|
@@ -642,7 +644,7 @@ module ExprNodes {
642644
)
643645
}
644646

645-
private class RequiredStringConstantValue extends RequiredConstantValue {
647+
private class RequiredStringlikeLiteralConstantValue extends RequiredConstantValue {
646648
override predicate requiredString(string s) {
647649
exists(StringlikeLiteralCfgNode n |
648650
s = getStringlikeLiteralCfgNodeValue(n) and
@@ -703,7 +705,7 @@ module ExprNodes {
703705
)
704706
}
705707

706-
private class RequiredStringConstantValue2 extends RequiredConstantValue {
708+
private class RequiredRexExpLiteralConstantValue extends RequiredConstantValue {
707709
override predicate requiredString(string s) { s = getRegExpLiteralCfgNodeValue(_) }
708710
}
709711

‎ruby/ql/test/library-tests/ast/ValueText.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,8 @@
367367
| literals/literals.rb:12:1:12:1 | 0 | 0 |
368368
| literals/literals.rb:13:1:13:5 | 0d900 | 0 |
369369
| literals/literals.rb:16:1:16:6 | 0x1234 | 4660 |
370+
| literals/literals.rb:17:1:17:10 | 0xdeadbeef | -559038737 |
371+
| literals/literals.rb:18:1:18:11 | 0xF00D_face | -267519282 |
370372
| literals/literals.rb:21:1:21:4 | 0123 | 83 |
371373
| literals/literals.rb:22:1:22:5 | 0o234 | 156 |
372374
| literals/literals.rb:23:1:23:6 | 0O45_6 | 302 |

‎ruby/ql/test/library-tests/ast/literals/literals.expected

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ allLiterals
1010
| literals.rb:12:1:12:1 | 0 | IntegerLiteral | 0 |
1111
| literals.rb:13:1:13:5 | 0d900 | IntegerLiteral | 0 |
1212
| literals.rb:16:1:16:6 | 0x1234 | IntegerLiteral | 4660 |
13-
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | <none> |
14-
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | <none> |
13+
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | -559038737 |
14+
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | -267519282 |
1515
| literals.rb:21:1:21:4 | 0123 | IntegerLiteral | 83 |
1616
| literals.rb:22:1:22:5 | 0o234 | IntegerLiteral | 156 |
1717
| literals.rb:23:1:23:6 | 0O45_6 | IntegerLiteral | 302 |
@@ -739,8 +739,8 @@ numericLiterals
739739
| literals.rb:12:1:12:1 | 0 | IntegerLiteral | 0 |
740740
| literals.rb:13:1:13:5 | 0d900 | IntegerLiteral | 0 |
741741
| literals.rb:16:1:16:6 | 0x1234 | IntegerLiteral | 4660 |
742-
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | <none> |
743-
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | <none> |
742+
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | -559038737 |
743+
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | -267519282 |
744744
| literals.rb:21:1:21:4 | 0123 | IntegerLiteral | 83 |
745745
| literals.rb:22:1:22:5 | 0o234 | IntegerLiteral | 156 |
746746
| literals.rb:23:1:23:6 | 0O45_6 | IntegerLiteral | 302 |
@@ -808,8 +808,8 @@ integerLiterals
808808
| literals.rb:12:1:12:1 | 0 | IntegerLiteral | 0 |
809809
| literals.rb:13:1:13:5 | 0d900 | IntegerLiteral | 0 |
810810
| literals.rb:16:1:16:6 | 0x1234 | IntegerLiteral | 4660 |
811-
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | <none> |
812-
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | <none> |
811+
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | -559038737 |
812+
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | -267519282 |
813813
| literals.rb:21:1:21:4 | 0123 | IntegerLiteral | 83 |
814814
| literals.rb:22:1:22:5 | 0o234 | IntegerLiteral | 156 |
815815
| literals.rb:23:1:23:6 | 0O45_6 | IntegerLiteral | 302 |

0 commit comments

Comments
 (0)