Skip to content

Lint shadow binding#1731

Merged
bakpakin merged 7 commits intomasterfrom
lint-shadow-binding
Mar 16, 2026
Merged

Lint shadow binding#1731
bakpakin merged 7 commits intomasterfrom
lint-shadow-binding

Conversation

@bakpakin
Copy link
Copy Markdown
Member

No description provided.

Prevent redefining bindings by accident. There are
a few cases where we want to allow this, such as the `default` macro, so
we allow a keyword :shadow to be included in the `def` expression to
turn off this lint.

TODO:
* Clean up test suite to remove binding shadowing
* Make sure that we don't get lints with *redef* turned on
* Add positive and negative tests for lint messages.
* Add location of shadowed binding in message
Why see the same message twice?
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds compiler linting for variable shadowing and updates the test suite/examples to either avoid shadowing (via renames) or explicitly mark intentional shadowing.

Changes:

  • Introduce shadowing checks during binding creation (def/var/function-related bindings) with an opt-out via :shadow metadata.
  • Refactor binding naming APIs (janetc_nameslot flags, namelocal def flags) and cache *redef* lookup in the compiler.
  • Update tests/examples to comply with the new lint and add initial shadow-lint coverage.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/core/compile.c Adds shadowing lint in janetc_nameslot and introduces janetc_shadowcheck; caches redef at compiler init.
src/core/compile.h Updates janetc_nameslot API, adds def flags and janetc_shadowcheck declaration, adds is_redef to compiler struct.
src/core/specials.c Threads def flags through local/global binding creation; supports :shadow metadata and avoids env insertion ordering issues.
src/boot/boot.janet Updates default macro to use :shadow; de-duplicates lints; modifies defmacro behavior.
test/helper.janet Flushes output more aggressively and updates assertion macros to avoid macro capture under shadowing rules.
test/suite-compile.janet Renames locals to avoid new lint collisions and adds basic tests for shadowing lint presence.
test/suite-vm.janet Renames a fiber binding to avoid shadowing.
test/suite-pp.janet Renames buffer temp binding to avoid shadowing.
test/suite-peg.janet Renames multiple grammar/p/peg bindings and parameters to avoid shadowing.
test/suite-parse.janet Renames parser bindings (pp0/p3/p4) to avoid shadowing.
test/suite-marsh.janet Uses :shadow metadata for intentional rebindings and renames to avoid shadowing lints.
test/suite-io.janet Uses :shadow and renames function arg to avoid shadowing an outer b.
test/suite-ev2.janet Uses :shadow for redefinitions in the second “variant” block.
test/suite-buffer.janet Renames buffer bindings to avoid shadowing.
test/suite-array.janet Renames array binding to avoid shadowing.
examples/life.janet Renames count binding to avoid shadowing.
examples/lazyseqs.janet Renames example APIs (delay/drop/take/empty?) to avoid conflicts/shadowing.
examples/iterate-fiber.janet Renames ff2 to avoid shadowing.
examples/error.janet Renames inner function argument to avoid shadowing outer x.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

(check-lint-compile '(def outer1 "b") "shadow global-to-global")
(check-lint-compile '(let [outer1 "b"] outer1) "shadow local-to-global")
(check-lint-compile '(do (def x "b") (def x "c")) "shadow local-to-local")

@@ -46,7 +46,6 @@
(defn defmacro :macro :flycheck
"Define a macro."
[name & more]
JanetSlot janetc_cslot(Janet x);

/* Search for a symbol */
/* Search for a symbol, and mark any found symbols as "used" for dead code elimintation and linting */
return 1;
}

/* Check if a binding is defined in an upper scope. This let's us check for
@bakpakin bakpakin merged commit df32109 into master Mar 16, 2026
32 checks passed
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 this pull request may close these issues.

2 participants