From fe2a4e15e57b8d0460d7f994c7411b4b2029b55d Mon Sep 17 00:00:00 2001 From: Morgante Pell Date: Tue, 22 Oct 2024 02:00:24 -0500 Subject: [PATCH] fix: improve shadow_scope (#242) --- ..._scope_shadow.md => _test_scope_shadow.md} | 55 +++++++++--------- .grit/patterns/js/replace_wildcard_imports.md | 2 +- .grit/patterns/js/shadow_scope.grit | 28 ++++++++++ .grit/patterns/js/variable_scoping.md | 56 +++++++++++++++++++ 4 files changed, 110 insertions(+), 31 deletions(-) rename .grit/patterns/js/{test_scope_shadow.md => _test_scope_shadow.md} (70%) create mode 100644 .grit/patterns/js/shadow_scope.grit create mode 100644 .grit/patterns/js/variable_scoping.md diff --git a/.grit/patterns/js/test_scope_shadow.md b/.grit/patterns/js/_test_scope_shadow.md similarity index 70% rename from .grit/patterns/js/test_scope_shadow.md rename to .grit/patterns/js/_test_scope_shadow.md index b2f722da..9017a212 100644 --- a/.grit/patterns/js/test_scope_shadow.md +++ b/.grit/patterns/js/_test_scope_shadow.md @@ -2,43 +2,16 @@ tags: [utility, test] --- -## Test shadow scope - -This tests the `shadows_identifier` pattern by finding all cases where a variable is shadowed. +## Shadow Identifiers +This tests the `scope_for_identifier` pattern by finding all cases where a variable is shadowed. ```grit language js -// Implementation -pattern shadows_identifier($name) { - or { - statement_block($statements) where { - $statements <: some variable($declarations) where { - $declarations <: contains variable_declarator(name=$name) - } - }, - arrow_function($parameters) where { - $parameters <: contains $name - }, - function_declaration($parameters) where { - $parameters <: contains $name - }, - for_in_statement() as $statement where { - $statement <: contains $name - }, - for_statement() as $statement where { - $statement <: contains $name - }, - `try { $_ } catch($catch) { $_ }` where { - $catch <: contains $name - }, - } -} - // Test case file($body) where { - $body <: contains bubble shadows_identifier(`x`) as $scope where { + $body <: contains bubble identifier_scope(`x`) as $scope where { $scope <: contains `x` => `shadowed` } } @@ -68,6 +41,28 @@ function notShadowingVar() { shadowingExample(); ``` +## Anonymous function variable definition + +```js +var shadowingExample = function (x) { + console.log(x); +}; +var notShadowingVar = function (y) { + console.log(y); +}; +shadowingExample(); +``` + +```js +var shadowingExample = function (shadowed) { + console.log(shadowed); +}; +var notShadowingVar = function (y) { + console.log(y); +}; +shadowingExample(); +``` + ## If statement ```js diff --git a/.grit/patterns/js/replace_wildcard_imports.md b/.grit/patterns/js/replace_wildcard_imports.md index 674597cd..683c2d32 100644 --- a/.grit/patterns/js/replace_wildcard_imports.md +++ b/.grit/patterns/js/replace_wildcard_imports.md @@ -30,7 +30,7 @@ pattern replace_wildcard_import() { `import * as $alias from $src` as $import where { $refs = [], $kept_refs = [], - $program <: contains used_alias($alias, $refs, $kept_refs) until shadows_identifier(name=$alias), + $program <: contains used_alias($alias, $refs, $kept_refs) until identifier_scope(name=$alias), $refs = distinct($refs), $joined_refs = join($refs, `, `), // Try the different scenarios diff --git a/.grit/patterns/js/shadow_scope.grit b/.grit/patterns/js/shadow_scope.grit new file mode 100644 index 00000000..00e4b5b2 --- /dev/null +++ b/.grit/patterns/js/shadow_scope.grit @@ -0,0 +1,28 @@ + +pattern identifier_scope($name) { + or { + statement_block($statements) where { + $statements <: some variable($declarations) where { + $declarations <: contains variable_declarator(name=$name) + } + }, + function($parameters) where { + $parameters <: contains $name + }, + arrow_function($parameters) where { + $parameters <: contains $name + }, + function_declaration($parameters) where { + $parameters <: contains $name + }, + for_in_statement() as $statement where { + $statement <: contains $name + }, + for_statement() as $statement where { + $statement <: contains $name + }, + `try { $_ } catch($catch) { $_ }` where { + $catch <: contains $name + }, + } +} diff --git a/.grit/patterns/js/variable_scoping.md b/.grit/patterns/js/variable_scoping.md new file mode 100644 index 00000000..ad01acb0 --- /dev/null +++ b/.grit/patterns/js/variable_scoping.md @@ -0,0 +1,56 @@ +--- +tags: + - docs + - full-examples +--- + +# Variable Scoping with `identifier_scope` + +Default Grit patterns are not generally aware of variable scoping, but you can use the `identifier_scope` pattern to find (or exclude) [scopes](https://developer.mozilla.org/en-US/docs/Glossary/Scope) where an identifier has been _locally_ defined. + +This is most often used when you want to target an import from a shared module but exclude scopes where the identifier is shadowed locally. + +For example, this pattern would rename `t` from the `translation` library to `translate` unless `t` is shadowed locally: + +```grit +language js + +`t` as $t => `translate` where { + $t <: imported_from(from=`"translation"`), + $t <: not within identifier_scope(name=`t`) +} +``` + +Here is a simple example file where `t` is shadowed locally: + +```js +import { t } from 'translation'; + +console.log(t('hello world')); + +function normal() { + console.log(t('hello world')); +} + +// t is an argument to this function, so the global t is not used and we should *not* rename it here. +function shadowed(t) { + console.log(t('hello world')); +} +``` + +When we rewrite it, the shadowed `t` is not renamed: + +```js +import { translate } from 'translation'; + +console.log(translate('hello world')); + +function normal() { + console.log(translate('hello world')); +} + +// t is an argument to this function, so the global t is not used and we should *not* rename it here. +function shadowed(t) { + console.log(t('hello world')); +} +```