Skip to content

Commit 2f2d72f

Browse files
committed
JS: Improve react-router support
1 parent 88932a4 commit 2f2d72f

File tree

5 files changed

+42
-8
lines changed

5 files changed

+42
-8
lines changed

javascript/ql/src/semmle/javascript/frameworks/React.qll

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -691,15 +691,22 @@ private DataFlow::SourceNode reactRouterDom() {
691691
result = DataFlow::moduleImport("react-router-dom")
692692
}
693693

694+
private DataFlow::SourceNode reactRouterMatchObject() {
695+
result = reactRouterDom().getAMemberCall(["useRouteMatch", "matchPath"])
696+
or
697+
exists(ReactComponent c |
698+
dependedOnByReactRouterClient(c.getTopLevel()) and
699+
result = c.getAPropRead("match")
700+
)
701+
}
702+
694703
private class ReactRouterSource extends ClientSideRemoteFlowSource {
695704
ClientSideRemoteFlowKind kind;
696705

697706
ReactRouterSource() {
698707
this = reactRouterDom().getAMemberCall("useParams") and kind.isPath()
699708
or
700-
exists(string prop |
701-
this = reactRouterDom().getAMemberCall("useRouteMatch").getAPropertyRead(prop)
702-
|
709+
exists(string prop | this = reactRouterMatchObject().getAPropertyRead(prop) |
703710
prop = "params" and kind.isPath()
704711
or
705712
prop = "url" and kind.isUrl()
@@ -713,16 +720,25 @@ private class ReactRouterSource extends ClientSideRemoteFlowSource {
713720

714721
/**
715722
* Holds if `mod` transitively depends on `react-router-dom`.
716-
*
717-
* We assume any React component in such a file may be used in a context where react-router
718-
* injects the `location` property in its `props` object.
719723
*/
720724
private predicate dependsOnReactRouter(Module mod) {
721725
mod.getAnImport().getImportedPath().getValue() = "react-router-dom"
722726
or
723727
dependsOnReactRouter(mod.getAnImportedModule())
724728
}
725729

730+
/**
731+
* Holds if `mod` is imported from a module that transitively depends on `react-router-dom`.
732+
*
733+
* We assume any React component in such a file may be used in a context where react-router
734+
* injects the `location` and `match` properties in its `props` object.
735+
*/
736+
private predicate dependedOnByReactRouterClient(Module mod) {
737+
dependsOnReactRouter(mod)
738+
or
739+
dependedOnByReactRouterClient(any(Module m | m.getAnImportedModule() = mod))
740+
}
741+
726742
/**
727743
* A reference to the DOM location obtained through `react-router-dom`
728744
*
@@ -740,7 +756,7 @@ private class ReactRouterLocationSource extends DOM::LocationSource::Range {
740756
this = reactRouterDom().getAMemberCall("useLocation")
741757
or
742758
exists(ReactComponent component |
743-
dependsOnReactRouter(component.getTopLevel()) and
759+
dependedOnByReactRouterClient(component.getTopLevel()) and
744760
this = component.getAPropRead("location")
745761
)
746762
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { MyComponent } from "./exportedComponent";
22

3-
export function render(color) {
3+
export function render({color, location}) {
44
return <MyComponent color={color}/>
55
}

javascript/ql/test/library-tests/frameworks/ReactJS/tests.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ test_ReactComponent_getInstanceMethod
1616
| es5.js:18:33:22:1 | {\\n ren ... ;\\n }\\n} | render | es5.js:19:11:21:3 | functio ... 1>;\\n } |
1717
| es6.js:1:1:8:1 | class H ... ;\\n }\\n} | render | es6.js:2:9:4:3 | () {\\n ... v>;\\n } |
1818
| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | render | exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} |
19+
| importedComponent.jsx:3:8:5:1 | functio ... or}/>\\n} | render | importedComponent.jsx:3:8:5:1 | functio ... or}/>\\n} |
1920
| plainfn.js:1:1:3:1 | functio ... div>;\\n} | render | plainfn.js:1:1:3:1 | functio ... div>;\\n} |
2021
| plainfn.js:5:1:7:1 | functio ... iv");\\n} | render | plainfn.js:5:1:7:1 | functio ... iv");\\n} |
2122
| plainfn.js:9:1:12:1 | functio ... rn x;\\n} | render | plainfn.js:9:1:12:1 | functio ... rn x;\\n} |
@@ -97,6 +98,7 @@ test_ReactComponent_ref
9798
| es6.js:14:1:20:1 | class H ... }\\n} | es6.js:17:9:17:12 | this |
9899
| es6.js:14:1:20:1 | class H ... }\\n} | es6.js:18:9:18:12 | this |
99100
| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | exportedComponent.jsx:1:8:1:7 | this |
101+
| importedComponent.jsx:3:8:5:1 | functio ... or}/>\\n} | importedComponent.jsx:3:8:3:7 | this |
100102
| namedImport.js:3:1:3:28 | class C ... nent {} | namedImport.js:3:27:3:26 | this |
101103
| namedImport.js:5:1:5:20 | class D extends C {} | namedImport.js:5:19:5:18 | this |
102104
| plainfn.js:1:1:3:1 | functio ... div>;\\n} | plainfn.js:1:1:1:0 | this |
@@ -198,6 +200,7 @@ test_ReactComponent_getADirectPropsSource
198200
| es6.js:1:1:8:1 | class H ... ;\\n }\\n} | es6.js:1:37:1:36 | args |
199201
| es6.js:1:1:8:1 | class H ... ;\\n }\\n} | es6.js:3:24:3:33 | this.props |
200202
| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | exportedComponent.jsx:1:29:1:33 | props |
203+
| importedComponent.jsx:3:8:5:1 | functio ... or}/>\\n} | importedComponent.jsx:3:24:3:40 | {color, location} |
201204
| namedImport.js:3:1:3:28 | class C ... nent {} | namedImport.js:3:27:3:26 | args |
202205
| namedImport.js:5:1:5:20 | class D extends C {} | namedImport.js:5:19:5:18 | args |
203206
| plainfn.js:1:1:3:1 | functio ... div>;\\n} | plainfn.js:1:16:1:20 | props |
@@ -236,6 +239,7 @@ test_ReactComponent
236239
| es6.js:1:1:8:1 | class H ... ;\\n }\\n} |
237240
| es6.js:14:1:20:1 | class H ... }\\n} |
238241
| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} |
242+
| importedComponent.jsx:3:8:5:1 | functio ... or}/>\\n} |
239243
| namedImport.js:3:1:3:28 | class C ... nent {} |
240244
| namedImport.js:5:1:5:20 | class D extends C {} |
241245
| plainfn.js:1:1:3:1 | functio ... div>;\\n} |
@@ -264,6 +268,8 @@ test_ReactComponent_getAPropRead
264268
| es5.js:18:33:22:1 | {\\n ren ... ;\\n }\\n} | name | es5.js:20:24:20:38 | this.props.name |
265269
| es6.js:1:1:8:1 | class H ... ;\\n }\\n} | name | es6.js:3:24:3:38 | this.props.name |
266270
| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | color | exportedComponent.jsx:2:32:2:42 | props.color |
271+
| importedComponent.jsx:3:8:5:1 | functio ... or}/>\\n} | color | importedComponent.jsx:3:25:3:29 | color |
272+
| importedComponent.jsx:3:8:5:1 | functio ... or}/>\\n} | location | importedComponent.jsx:3:32:3:39 | location |
267273
| plainfn.js:1:1:3:1 | functio ... div>;\\n} | name | plainfn.js:2:22:2:31 | props.name |
268274
| preact.js:1:1:7:1 | class H ... }\\n} | name | preact.js:3:9:3:18 | props.name |
269275
| probably-a-component.js:1:1:6:1 | class H ... }\\n} | name | probably-a-component.js:3:9:3:23 | this.props.name |
@@ -288,6 +294,9 @@ test_JSXname
288294
| thisAccesses.js:60:19:60:41 | <this.n ... s.name> | thisAccesses.js:60:20:60:28 | this.name | this.name | dot |
289295
| thisAccesses.js:61:19:61:41 | <this.t ... s.this> | thisAccesses.js:61:20:61:28 | this.this | this.this | dot |
290296
| thisAccesses_importedMappers.js:13:16:13:21 | <div/> | thisAccesses_importedMappers.js:13:17:13:19 | div | div | Identifier |
297+
| use-react-router.jsx:5:17:5:87 | <Router ... Router> | use-react-router.jsx:5:18:5:23 | Router | Router | Identifier |
298+
| use-react-router.jsx:5:25:5:78 | <Route> ... /Route> | use-react-router.jsx:5:26:5:30 | Route | Route | Identifier |
299+
| use-react-router.jsx:5:32:5:70 | <Import ... ponent> | use-react-router.jsx:5:33:5:49 | ImportedComponent | ImportedComponent | Identifier |
291300
| useHigherOrderComponent.jsx:5:12:5:39 | <SomeCo ... "red"/> | useHigherOrderComponent.jsx:5:13:5:25 | SomeComponent | SomeComponent | Identifier |
292301
| useHigherOrderComponent.jsx:11:12:11:46 | <LazyLo ... lazy"/> | useHigherOrderComponent.jsx:11:13:11:31 | LazyLoadedComponent | LazyLoadedComponent | Identifier |
293302
| useHigherOrderComponent.jsx:17:12:17:48 | <LazyLo ... azy2"/> | useHigherOrderComponent.jsx:17:13:17:32 | LazyLoadedComponent2 | LazyLoadedComponent2 | Identifier |
@@ -298,3 +307,5 @@ test_JSXName_this
298307
| statePropertyWrites.js:38:12:38:45 | <div>He ... }</div> | statePropertyWrites.js:38:24:38:27 | this |
299308
| thisAccesses.js:60:19:60:41 | <this.n ... s.name> | thisAccesses.js:60:20:60:23 | this |
300309
| thisAccesses.js:61:19:61:41 | <this.t ... s.this> | thisAccesses.js:61:20:61:23 | this |
310+
locationSource
311+
| importedComponent.jsx:3:32:3:39 | location |

javascript/ql/test/library-tests/frameworks/ReactJS/tests.ql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,5 @@ import ReactComponent_getACandidatePropsValue
99
import ReactComponent
1010
import ReactComponent_getAPropRead
1111
import ReactName
12+
13+
query DataFlow::SourceNode locationSource() { result = DOM::locationSource() }
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import * as ReactDOM from "react-dom"
2+
import { Route, Router } from "react-router-dom";
3+
import ImportedComponent from "./importedComponent";
4+
5+
ReactDOM.render(<Router><Route><ImportedComponent></ImportedComponent></Route></Router>);

0 commit comments

Comments
 (0)