Skip to content

JavaScript: Fix modelling of _.partial.#386

Merged
semmle-qlci merged 1 commit into
github:masterfrom
xiemaisi:js/lodash_partial
Nov 1, 2018
Merged

JavaScript: Fix modelling of _.partial.#386
semmle-qlci merged 1 commit into
github:masterfrom
xiemaisi:js/lodash_partial

Conversation

@xiemaisi
Copy link
Copy Markdown

Like Function.prototype.bind (but unlike ramda.partial) it takes the curried arguments as rest arguments, not as an array; cf. https://lodash.com/docs/4.17.10#partial and https://underscorejs.org/#partial.

@xiemaisi xiemaisi added the JS label Oct 31, 2018
@xiemaisi xiemaisi requested a review from a team as a code owner October 31, 2018 01:58
Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except for the benign test failure.

@@ -1,13 +1,13 @@
WARNING: Predicate flowsFrom has been deprecated and may be removed in future (ReflectedXssWithCustomSanitizer_old.ql:21,11-20)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jenkins failure: you need to use a different version of /code to get the samme warning order.

@asger-semmle
Copy link
Copy Markdown
Contributor

Thanks for catching that. I thought I tested it, but I think I was tricked by a coercion:

function f(x,y) { console.log(x + y) }
_.partial(f, ["hello"])("world")

Like `Function.prototype.bind` (but unlike `ramda.partial`) it takes the curried arguments as rest arguments, not as an array;
cf. https://lodash.com/docs/4.17.10#partial and https://underscorejs.org/#partial.
@xiemaisi
Copy link
Copy Markdown
Author

Amended and added a change note.

@semmle-qlci semmle-qlci merged commit 0883346 into github:master Nov 1, 2018
@xiemaisi xiemaisi deleted the js/lodash_partial branch November 5, 2018 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants