Skip to content

Commit 80c20cb

Browse files
authored
Merge pull request github#3297 from asger-semmle/js/isambient-refactor
Approved by esbena
2 parents d75d520 + ca60e82 commit 80c20cb

File tree

6 files changed

+50
-47
lines changed

6 files changed

+50
-47
lines changed

javascript/ql/src/semmle/javascript/AST.qll

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,42 @@ class ASTNode extends @ast_node, Locatable {
125125
/** Holds if this syntactic entity belongs to an externs file. */
126126
predicate inExternsFile() { getTopLevel().isExterns() }
127127

128+
/**
129+
* Holds if this is an ambient node that is not a `TypeExpr` and is not inside a `.d.ts` file
130+
*
131+
* Since the overwhelming majority of ambient nodes are `TypeExpr` or inside `.d.ts` files,
132+
* we avoid caching them.
133+
*/
134+
cached
135+
private predicate isAmbientInternal() {
136+
getParent().isAmbientInternal()
137+
or
138+
not isAmbientTopLevel(getTopLevel()) and
139+
(
140+
this instanceof ExternalModuleDeclaration
141+
or
142+
this instanceof GlobalAugmentationDeclaration
143+
or
144+
this instanceof ExportAsNamespaceDeclaration
145+
or
146+
this instanceof TypeAliasDeclaration
147+
or
148+
this instanceof InterfaceDeclaration
149+
or
150+
hasDeclareKeyword(this)
151+
or
152+
hasTypeKeyword(this)
153+
or
154+
// An export such as `export declare function f()` should be seen as ambient.
155+
hasDeclareKeyword(this.(ExportNamedDeclaration).getOperand())
156+
or
157+
exists(Function f |
158+
this = f and
159+
not f.hasBody()
160+
)
161+
)
162+
}
163+
128164
/**
129165
* Holds if this is part of an ambient declaration or type annotation in a TypeScript file.
130166
*
@@ -134,9 +170,22 @@ class ASTNode extends @ast_node, Locatable {
134170
* The TypeScript compiler emits no code for ambient declarations, but they
135171
* can affect name resolution and type checking at compile-time.
136172
*/
137-
predicate isAmbient() { getParent().isAmbient() }
173+
pragma[inline]
174+
predicate isAmbient() {
175+
isAmbientInternal()
176+
or
177+
isAmbientTopLevel(getTopLevel())
178+
or
179+
this instanceof TypeExpr
180+
}
138181
}
139182

183+
/**
184+
* Holds if the given file is a `.d.ts` file.
185+
*/
186+
cached
187+
private predicate isAmbientTopLevel(TopLevel tl) { tl.getFile().getBaseName().matches("%.d.ts") }
188+
140189
/**
141190
* A toplevel syntactic unit; that is, a stand-alone script, an inline script
142191
* embedded in an HTML `<script>` tag, a code snippet assigned to an HTML event
@@ -197,11 +246,6 @@ class TopLevel extends @toplevel, StmtContainer {
197246
override ControlFlowNode getFirstControlFlowNode() { result = getEntry() }
198247

199248
override string toString() { result = "<toplevel>" }
200-
201-
override predicate isAmbient() {
202-
getFile().getFileType().isTypeScript() and
203-
getFile().getBaseName().matches("%.d.ts")
204-
}
205249
}
206250

207251
/**

javascript/ql/src/semmle/javascript/Classes.qll

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,12 +1059,6 @@ class FieldDeclaration extends MemberDeclaration, @field {
10591059

10601060
/** Holds if this is a TypeScript field marked as definitely assigned with the `!` operator. */
10611061
predicate hasDefiniteAssignmentAssertion() { hasDefiniteAssignmentAssertion(this) }
1062-
1063-
override predicate isAmbient() {
1064-
hasDeclareKeyword(this)
1065-
or
1066-
getParent().isAmbient()
1067-
}
10681062
}
10691063

10701064
/**

javascript/ql/src/semmle/javascript/ES2015Modules.qll

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,6 @@ class ImportDeclaration extends Stmt, Import, @importdeclaration {
7979

8080
/** Holds if this is declared with the `type` keyword, so it only imports types. */
8181
predicate isTypeOnly() { hasTypeKeyword(this) }
82-
83-
override predicate isAmbient() {
84-
Stmt.super.isAmbient() or
85-
isTypeOnly()
86-
}
8782
}
8883

8984
/** A literal path expression appearing in an `import` declaration. */
@@ -267,11 +262,6 @@ abstract class ExportDeclaration extends Stmt, @exportdeclaration {
267262

268263
/** Holds if is declared with the `type` keyword, so only types are exported. */
269264
predicate isTypeOnly() { hasTypeKeyword(this) }
270-
271-
override predicate isAmbient() {
272-
Stmt.super.isAmbient() or
273-
isTypeOnly()
274-
}
275265
}
276266

277267
/**
@@ -422,11 +412,6 @@ class ExportNamedDeclaration extends ExportDeclaration, @exportnameddeclaration
422412

423413
/** Gets an export specifier of this declaration. */
424414
ExportSpecifier getASpecifier() { result = getSpecifier(_) }
425-
426-
override predicate isAmbient() {
427-
// An export such as `export declare function f()` should be seen as ambient.
428-
hasDeclareKeyword(getOperand()) or getParent().isAmbient()
429-
}
430415
}
431416

432417
/**

javascript/ql/src/semmle/javascript/Functions.qll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,8 +397,6 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
397397
*/
398398
predicate isAbstract() { exists(MethodDeclaration md | this = md.getBody() | md.isAbstract()) }
399399

400-
override predicate isAmbient() { getParent().isAmbient() or not hasBody() }
401-
402400
/**
403401
* Holds if this function cannot be invoked using `new` because it
404402
* is of the given `kind`.

javascript/ql/src/semmle/javascript/Stmt.qll

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,6 @@ class Stmt extends @stmt, ExprOrStmt, Documentable {
5454
getContainer().(Expr).getEnclosingStmt().nestedIn(outer)
5555
}
5656

57-
override predicate isAmbient() { hasDeclareKeyword(this) or getParent().isAmbient() }
58-
5957
/**
6058
* Gets the `try` statement with a catch block containing this statement without
6159
* crossing function boundaries or other `try ` statements with catch blocks.
@@ -931,8 +929,6 @@ class DebuggerStmt extends @debuggerstmt, Stmt {
931929
*/
932930
class FunctionDeclStmt extends @functiondeclstmt, Stmt, Function {
933931
override Stmt getEnclosingStmt() { result = this }
934-
935-
override predicate isAmbient() { Function.super.isAmbient() }
936932
}
937933

938934
/**

javascript/ql/src/semmle/javascript/TypeScript.qll

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,6 @@ class ExternalModuleDeclaration extends Stmt, StmtContainer, @externalmoduledecl
155155
int getNumStmt() { result = count(getAStmt()) }
156156

157157
override StmtContainer getEnclosingContainer() { result = this.getContainer() }
158-
159-
override predicate isAmbient() { any() }
160158
}
161159

162160
/**
@@ -176,8 +174,6 @@ class GlobalAugmentationDeclaration extends Stmt, StmtContainer, @globalaugmenta
176174
int getNumStmt() { result = count(getAStmt()) }
177175

178176
override StmtContainer getEnclosingContainer() { result = this.getContainer() }
179-
180-
override predicate isAmbient() { any() }
181177
}
182178

183179
/** A TypeScript "import-equals" declaration. */
@@ -237,8 +233,6 @@ class ExportAsNamespaceDeclaration extends Stmt, @exportasnamespacedeclaration {
237233
* Gets the `X` in `export as namespace X`.
238234
*/
239235
Identifier getIdentifier() { result = getChildExpr(0) }
240-
241-
override predicate isAmbient() { any() }
242236
}
243237

244238
/**
@@ -259,8 +253,6 @@ class TypeAliasDeclaration extends @typealiasdeclaration, TypeParameterized, Stm
259253

260254
override string describe() { result = "type alias " + getName() }
261255

262-
override predicate isAmbient() { any() }
263-
264256
/**
265257
* Gets the canonical name of the type being defined.
266258
*/
@@ -286,8 +278,6 @@ class InterfaceDeclaration extends Stmt, InterfaceDefinition, @interfacedeclarat
286278

287279
override StmtContainer getContainer() { result = Stmt.super.getContainer() }
288280

289-
override predicate isAmbient() { any() }
290-
291281
override string describe() { result = "interface " + getName() }
292282

293283
/**
@@ -533,8 +523,6 @@ class LocalNamespaceName extends @local_namespace_name, LexicalName {
533523
class TypeExpr extends ExprOrType, @typeexpr, TypeAnnotation {
534524
override string toString() { typeexprs(this, _, _, _, result) }
535525

536-
override predicate isAmbient() { any() }
537-
538526
/**
539527
* Gets the static type expressed by this type annotation.
540528
*
@@ -1410,8 +1398,6 @@ class EnumDeclaration extends NamespaceDefinition, @enumdeclaration, AST::ValueN
14101398
/** Holds if this enumeration is declared with the `const` keyword. */
14111399
predicate isConst() { isConstEnum(this) }
14121400

1413-
override predicate isAmbient() { hasDeclareKeyword(this) or getParent().isAmbient() }
1414-
14151401
override ControlFlowNode getFirstControlFlowNode() { result = getIdentifier() }
14161402
}
14171403

0 commit comments

Comments
 (0)