Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double colon stubbed function name + double colon assignment = cryptic parsing bug #23

Closed
kjohnsen opened this issue Jun 26, 2018 · 2 comments · Fixed by #31
Closed

Comments

@kjohnsen
Copy link
Contributor

kjohnsen commented Jun 26, 2018

I wish I could have crafted a more comprehensible title, but this is a weird bug. Here's a reproducible example:

library(testthat)
library(mockery)

myFunc <- function() {
  x <- c(a=42)
  names(x) # this works fine
  names(x) <- 'b' # so does this
  base::names(x) # and this
  base::names(x) <- 'b' # <--------------------------------------- here it breaks
  return(42)
}

test_that('most stubs work fine', {
  stub(myFunc, 'singleColon:trash_works_fine', TRUE)
  expect_equal(myFunc(), 42)
})

test_that('double colon in stubbed function breaks', {
  stub(myFunc, 'doubleColon::triggers_cryptic_bug', TRUE)
  expect_equal(myFunc(), 42)
})

The indicated line produces the following error:

Error: Test failed: 'double colon in mocked function breaks'
* <text>:2:0: unexpected end of input
1: base::names<-
   ^
1: expect_equal(myFunc(), 42) at ~/.active-rstudio-document:21
2: quasi_label(enquo(object), label)
3: eval_bare(get_expr(quo), get_env(quo))
4: myFunc()
5: base::`names<-` at ~/.active-rstudio-document:9
6: eval(parse(text = code), eval_env)
7: parse(text = code)

I found this bug in testing my own package, where another <package>::function<- statement (not base::names<-) gave me the same "unexpected end of input" error. It took me a while to even figure out it was mockery that was causing the problem, since it doesn't appear anywhere in the stacktrace.

@nsfinkelstein
Copy link
Contributor

@kjohnsen

Thanks for finding this bug and writing it up. It turns out to happen whenever something is assigned to a function call. I haven't quite figured out why, except that something about how we're mangling names allows the original function to be called, but not called then assigned to. To be honest I've never been a fan of assigning to function calls.

I don't have the bandwidth to figure out why this is broken and fix it. If you do, please feel free. Otherwise, I certainly agree that at least the error should be clearer in specifying that double colon stubbed functions cannot be assigned to.

If you are able to create a pull request to that effect, it would be appreciated.

@kjohnsen
Copy link
Contributor Author

Thanks for getting back. I wish I had time to work on a pull request, but I don't in the near future :(

Meanwhile I'm working around this by just importing the function into my package namespace so I don't have to use the double colon.

jimhester added a commit that referenced this issue Aug 29, 2019
The parsing was failing because `foo::bar<-` is not parse-able, however

    foo::`bar<-`

Is parse-able, so that is what we do now. A related issue was the
mangling was creating names like `fooXXXbar<-`, which again is not
parse-able, so that was changed to quote the whole name with backticks.

Fixes #23
jimhester added a commit that referenced this issue Aug 29, 2019
The parsing was failing because `foo::bar<-` is not parse-able, however

    foo::`bar<-`

Is parse-able, so that is what we do now. A related issue was the
mangling was creating names like `fooXXXbar<-`, which again is not
parse-able, so that was changed to quote the whole name with backticks.

Fixes #23
jimhester added a commit that referenced this issue Aug 29, 2019
The parsing was failing because `foo::bar<-` is not parse-able, however

    foo::`bar<-`

Is parse-able, so that is what we do now. A related issue was the
mangling was creating names like `fooXXXbar<-`, which again is not
parse-able, so that was changed to quote the whole name with backticks.

Fixes #23
jimhester added a commit that referenced this issue Aug 29, 2019
The parsing was failing because `foo::bar<-` is not parse-able, however

    foo::`bar<-`

Is parse-able, so that is what we do now. A related issue was the
mangling was creating names like `fooXXXbar<-`, which again is not
parse-able, so that was changed to quote the whole name with backticks.

Fixes #23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants