From 4367328e9e3c72aac7f1c8503144d89c40d20335 Mon Sep 17 00:00:00 2001 From: Romain Francois Date: Wed, 28 Oct 2015 15:47:40 +0100 Subject: [PATCH] only allow global() --- inst/include/dplyr/Result/GroupedCallProxy.h | 5 ++++- src/api.cpp | 4 +++- tests/testthat/test-mutate.r | 8 ++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/inst/include/dplyr/Result/GroupedCallProxy.h b/inst/include/dplyr/Result/GroupedCallProxy.h index 9859e76a93..f0172f9026 100644 --- a/inst/include/dplyr/Result/GroupedCallProxy.h +++ b/inst/include/dplyr/Result/GroupedCallProxy.h @@ -97,6 +97,7 @@ namespace dplyr { if( TYPEOF(obj) == LANGSXP && CAR(obj) == Rf_install("global") ){ SEXP symb = CADR(obj) ; + if( TYPEOF(symb) != SYMSXP ) stop( "global only handles symbols" ) ; SEXP res = env.find(CHAR(PRINTNAME(symb))) ; call = res ; return ; @@ -107,8 +108,10 @@ namespace dplyr { switch( TYPEOF( head ) ){ case LANGSXP: - if( CAR(head) == Rf_install("global") && TYPEOF(CADR(head)) == SYMSXP ){ + if( CAR(head) == Rf_install("global") ){ SEXP symb = CADR(head) ; + if( TYPEOF(symb) != SYMSXP ) stop( "global only handles symbols" ) ; + SEXP res = env.find( CHAR(PRINTNAME(symb)) ) ; SETCAR(obj, res) ; diff --git a/src/api.cpp b/src/api.cpp index dd638de764..844ed3b766 100644 --- a/src/api.cpp +++ b/src/api.cpp @@ -76,6 +76,7 @@ namespace dplyr{ if( TYPEOF(obj) == LANGSXP && CAR(obj) == Rf_install("global") ){ SEXP symb = CADR(obj) ; + if( TYPEOF(symb) != SYMSXP ) stop( "global only handles symbols" ) ; SEXP res = env.find(CHAR(PRINTNAME(symb))) ; call = res ; return ; @@ -85,8 +86,9 @@ namespace dplyr{ SEXP head = CAR(obj) ; switch( TYPEOF( head ) ){ case LANGSXP: - if( CAR(head) == Rf_install("global") && TYPEOF(CADR(head)) == SYMSXP ){ + if( CAR(head) == Rf_install("global") ){ SEXP symb = CADR(head) ; + if( TYPEOF(symb) != SYMSXP ) stop( "global only handles symbols" ) ; SEXP res = env.find( CHAR(PRINTNAME(symb)) ) ; SETCAR(obj, res) ; diff --git a/tests/testthat/test-mutate.r b/tests/testthat/test-mutate.r index 77669071b7..894c57521d 100644 --- a/tests/testthat/test-mutate.r +++ b/tests/testthat/test-mutate.r @@ -443,15 +443,23 @@ test_that("mutate recognizes global #1469", { vs <- 4 res <- mtcars %>% mutate(a = global(vs)) expect_true( all(res$a == 4) ) + expect_error( mtcars %>% mutate(global("vs")), "global only handles symbols" ) res <- mtcars %>% mutate(a = global(vs) + 1) expect_true( all(res$a == 5) ) + expect_error( mtcars %>% mutate(global("vs") + 1), "global only handles symbols" ) res <- mtcars %>% mutate(a = 1+global(vs) ) expect_true( all(res$a == 5) ) + expect_error( mtcars %>% mutate(1 + global("vs")), "global only handles symbols" ) res <- mtcars %>% group_by(cyl) %>% mutate(a = global(vs)) expect_true( all(res$a == 4) ) + expect_error( mtcars %>% group_by(cyl) %>% mutate(a = global("vs")), "global only handles symbols" ) res <- mtcars %>% group_by(cyl) %>% mutate(a = global(vs)+1) expect_true( all(res$a == 5) ) + expect_error( mtcars %>% group_by(cyl) %>% mutate(a = global("vs") + 1), "global only handles symbols" ) + res <- mtcars %>% group_by(cyl) %>% mutate(a = 1+global(vs)) expect_true( all(res$a == 5) ) + expect_error( mtcars %>% group_by(cyl) %>% mutate(a = 1 + global("vs")), "global only handles symbols" ) + })