search

beanshell / beanshell

Beanshell scripting language
Apache License 2.0
819 stars 183 forks source link

Fix cloneable support. #678

Closed nickl- closed 1 year ago

nickl- commented 1 year ago

Fixes #421 which originally reported the problems with cloning scripted objects. Had to deep copy all the contexts and most references to finally be able to produce two independent instances after a clone. Also completed the existing attempt to implement cloning, and we now return an actual instance instead of just copied This namespace.

codecov[bot] commented 1 year ago

Codecov Report

Merging #678 (8b6cbfc) into master (e42df53) will increase coverage by 0.04%. The diff coverage is 88.88%.

@@             Coverage Diff              @@
##             master     #678      +/-   ##
============================================
+ Coverage     71.27%   71.31%   +0.04%     
- Complexity     2791     2796       +5     
============================================
  Files           106      106              
  Lines          9087     9121      +34     
  Branches       1772     1776       +4     
============================================
+ Hits           6477     6505      +28     
- Misses         2199     2204       +5     
- Partials        411      412       +1
Flag Coverage Δ
unittests 71.31% <88.88%> (+0.04%) :arrow_up:

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

Impacted Files Coverage Δ
src/main/java/bsh/NameSpace.java 86.45% <50.00%> (+0.02%) :arrow_up:
src/main/java/bsh/BshMethod.java 73.91% <88.88%> (+0.77%) :arrow_up:
src/main/java/bsh/This.java 80.45% <91.17%> (+1.30%) :arrow_up:
src/main/java/bsh/ExternalNameSpace.java 60.52% <0.00%> (-10.53%) :arrow_down:
.../main/java/bsh/engine/ScriptContextEngineView.java 54.83% <0.00%> (-3.23%) :arrow_down:
src/main/java/bsh/LHS.java 69.62% <0.00%> (+0.63%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

nickl- commented 1 year ago

Scrutinizer insists that we should remove the clone I added on bshmethod. There is some poetic justice in the thought that when implementing clone you will at least add one cloneable class.

This patch already took way more time then I initially expected, the methods need to copied or they simply produce garbage, and yes clone is a messed up api and we are likely going to see more issues on this in the future. I will probably write a manual copy for methods at some point but right now it seems like too much effort and no reward. The clone is working for now...