Skip to content

Commit b8bfdcc

Browse files
committed
improve performance in ServiceDefinitions by inlining, and refactoring away a SourceNode
1 parent 93bcc37 commit b8bfdcc

File tree

1 file changed

+22
-27
lines changed

1 file changed

+22
-27
lines changed

javascript/ql/src/semmle/javascript/frameworks/AngularJS/ServiceDefinitions.qll

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -297,34 +297,29 @@ abstract private class CustomSpecialServiceDefinition extends CustomServiceDefin
297297
/**
298298
* Holds if `mce` defines a service of type `moduleMethodName` with name `serviceName` using the `factoryFunction` as the factory function.
299299
*/
300+
bindingset[moduleMethodName]
300301
private predicate isCustomServiceDefinitionOnModule(
301302
DataFlow::CallNode mce, string moduleMethodName, string serviceName,
302-
DataFlow::SourceNode factoryFunction
303+
DataFlow::SourceNode factoryArgument
303304
) {
304305
mce = moduleRef(_).getAMethodCall(moduleMethodName) and
305-
(
306-
moduleMethodName = "controller" or
307-
moduleMethodName = "filter" or
308-
moduleMethodName = "directive" or
309-
moduleMethodName = "component" or
310-
moduleMethodName = "animation"
311-
) and
312306
mce.getArgument(0).asExpr().mayHaveStringValue(serviceName) and
313-
factoryFunction.flowsTo(mce.getArgument(1))
307+
factoryArgument = mce.getArgument(1)
314308
}
315309

310+
pragma[inline]
316311
private predicate isCustomServiceDefinitionOnProvider(
317312
DataFlow::CallNode mce, string providerName, string providerMethodName, string serviceName,
318-
DataFlow::SourceNode factoryFunction
313+
DataFlow::Node factoryArgument
319314
) {
320315
mce = builtinServiceRef(providerName).getAMethodCall(providerMethodName) and
321316
(
322317
mce.getNumArgument() = 1 and
323-
factoryFunction.flowsTo(mce.getOptionArgument(0, serviceName))
318+
factoryArgument = mce.getOptionArgument(0, serviceName)
324319
or
325320
mce.getNumArgument() = 2 and
326321
mce.getArgument(0).asExpr().mayHaveStringValue(serviceName) and
327-
factoryFunction.flowsTo(mce.getArgument(1))
322+
factoryArgument = mce.getArgument(1)
328323
)
329324
}
330325

@@ -333,7 +328,7 @@ private predicate isCustomServiceDefinitionOnProvider(
333328
*/
334329
class ControllerDefinition extends CustomSpecialServiceDefinition {
335330
string name;
336-
DataFlow::SourceNode factoryFunction;
331+
DataFlow::Node factoryFunction;
337332

338333
ControllerDefinition() {
339334
isCustomServiceDefinitionOnModule(this, "controller", name, factoryFunction) or
@@ -343,17 +338,17 @@ class ControllerDefinition extends CustomSpecialServiceDefinition {
343338

344339
override string getName() { result = name }
345340

346-
override DataFlow::SourceNode getAService() { result = factoryFunction }
341+
override DataFlow::SourceNode getAService() { result = factoryFunction.getALocalSource() }
347342

348-
override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction }
343+
override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction.getALocalSource() }
349344
}
350345

351346
/**
352347
* A filter defined with `module.filter` or `$filterProvider.register`.
353348
*/
354349
class FilterDefinition extends CustomSpecialServiceDefinition {
355350
string name;
356-
DataFlow::SourceNode factoryFunction;
351+
DataFlow::Node factoryFunction;
357352

358353
FilterDefinition() {
359354
isCustomServiceDefinitionOnModule(this, "filter", name, factoryFunction) or
@@ -364,20 +359,20 @@ class FilterDefinition extends CustomSpecialServiceDefinition {
364359

365360
override DataFlow::SourceNode getAService() {
366361
exists(InjectableFunction f |
367-
f = factoryFunction and
362+
f = factoryFunction.getALocalSource() and
368363
result.flowsToExpr(f.asFunction().getAReturnedExpr())
369364
)
370365
}
371366

372-
override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction }
367+
override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction.getALocalSource() }
373368
}
374369

375370
/**
376371
* A directive defined with `module.directive` or `$compileProvider.directive`.
377372
*/
378373
class DirectiveDefinition extends CustomSpecialServiceDefinition {
379374
string name;
380-
DataFlow::SourceNode factoryFunction;
375+
DataFlow::Node factoryFunction;
381376

382377
DirectiveDefinition() {
383378
isCustomServiceDefinitionOnModule(this, "directive", name, factoryFunction) or
@@ -393,7 +388,7 @@ class DirectiveDefinition extends CustomSpecialServiceDefinition {
393388
)
394389
}
395390

396-
override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction }
391+
override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction.getALocalSource() }
397392
}
398393

399394
private class CustomDirectiveControllerDependencyInjection extends DependencyInjection {
@@ -416,7 +411,7 @@ private class CustomDirectiveControllerDependencyInjection extends DependencyInj
416411
*/
417412
class ComponentDefinition extends CustomSpecialServiceDefinition {
418413
string name;
419-
DataFlow::SourceNode config;
414+
DataFlow::Node config;
420415

421416
ComponentDefinition() {
422417
isCustomServiceDefinitionOnModule(this, "component", name, config) or
@@ -435,15 +430,15 @@ class ComponentDefinition extends CustomSpecialServiceDefinition {
435430
override DataFlow::SourceNode getAFactoryFunction() { none() }
436431

437432
/** Gets the configuration object for the defined component. */
438-
DataFlow::SourceNode getConfig() { result = config }
433+
DataFlow::SourceNode getConfig() { result = config.getALocalSource() }
439434
}
440435

441436
/**
442437
* An animation defined with `module.animation` or `$animationProvider.register`.
443438
*/
444439
class AnimationDefinition extends CustomSpecialServiceDefinition {
445440
string name;
446-
DataFlow::SourceNode factoryFunction;
441+
DataFlow::Node factoryFunction;
447442

448443
AnimationDefinition() {
449444
isCustomServiceDefinitionOnModule(this, "animation", name, factoryFunction) or
@@ -454,12 +449,12 @@ class AnimationDefinition extends CustomSpecialServiceDefinition {
454449

455450
override DataFlow::SourceNode getAService() {
456451
exists(InjectableFunction f |
457-
f = factoryFunction and
452+
f = factoryFunction.getALocalSource() and
458453
result.flowsToExpr(f.asFunction().getAReturnedExpr())
459454
)
460455
}
461456

462-
override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction }
457+
override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction.getALocalSource() }
463458
}
464459

465460
/**
@@ -500,7 +495,7 @@ private class LinkFunctionWithScopeInjection extends ServiceRequest {
500495
class InjectableFunctionServiceRequest extends ServiceRequest {
501496
InjectableFunction injectedFunction;
502497

503-
InjectableFunctionServiceRequest() { DataFlow::valueNode(this) = injectedFunction }
498+
InjectableFunctionServiceRequest() { injectedFunction.getAstNode() = this }
504499

505500
/**
506501
* Gets the function of this request.
@@ -589,7 +584,7 @@ class ServiceRecipeDefinition extends RecipeDefinition {
589584

590585
exists(InjectableFunction f |
591586
f = getAFactoryFunction() and
592-
result = DataFlow::valueNode(f.asFunction())
587+
result.getAstNode() = f.asFunction()
593588
)
594589
}
595590
}

0 commit comments

Comments
 (0)