clarinet icon indicating copy to clipboard operation
clarinet copied to clipboard

fix: check for reserved names in let bindings

Open obycode opened this issue 3 years ago • 3 comments

This change checks for reserved names in addition to checking for names previously defined by the user.

Fixes: #418

obycode avatar Jun 21 '22 21:06 obycode

Codecov Report

Merging #420 (0a3f56a) into develop (3fc8ac2) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #420      +/-   ##
===========================================
+ Coverage    22.31%   22.33%   +0.01%     
===========================================
  Files          163      163              
  Lines        39186    39187       +1     
===========================================
+ Hits          8745     8753       +8     
+ Misses       30441    30434       -7     
Flag Coverage Δ
unittests 22.33% <100.00%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...repl/src/clarity/analysis/type_checker/contexts.rs 80.54% <100.00%> (+0.10%) :arrow_up:
...ents/clarity-repl/src/repl/ast/parser/lexer/mod.rs 36.13% <0.00%> (+0.35%) :arrow_up:
...larity-repl/src/clarity/ast/traits_resolver/mod.rs 64.28% <0.00%> (+0.79%) :arrow_up:
...ponents/clarity-repl/src/clarity/util/secp256k1.rs 12.06% <0.00%> (+0.86%) :arrow_up:
components/clarinet-files/src/project_manifest.rs 76.28% <0.00%> (+3.09%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3fc8ac2...0a3f56a. Read the comment docs.

codecov-commenter avatar Jun 21 '22 21:06 codecov-commenter

  • [ ] I should also add unit tests for this before merging

obycode avatar Jun 22 '22 14:06 obycode

  • [ ] I should also add unit tests for this before merging

Yep! And depending on how the test(s) will look like, it might make sense to turn the test(s) into (a single?) proptest. (May be tackled on a separate PR.)

moodmosaic avatar Jun 23 '22 08:06 moodmosaic

With the VM being pulled from the stacks-blockchain repo this approach does not seem to be viable anymore, so I'll close this PR.

lgalabru avatar Sep 30 '22 20:09 lgalabru