[feat] added a default scope manager

[fishonamos]

Modified the code so when the context is created, the Scope Manager is also initialised. This will solve #345

[TAdev0]

@fishonamos hi, your code will not compile because NewScopeManager takes a map[string]any as argument. What you can do instead is calling *hinter.DefaultNewScopeManager() , and DefaultNewScopeManager will call NewScopeManager for you passing an empty array as argument.

[TAdev0]

Basically, you have 2 possibilities:

• manually defining the ScopeManager field of ctx

  ctx := &hinter.HintRunnerContext{
          ScopeManager: *hinter.DefaultNewScopeManager(),
      }

• creating an uninitialized ctx an calling InitializeScopeManager , passing ctx and an empty array of string to initialize the ScopeManager field:

  
      ctx := &hinter.HintRunnerContext{}
      hinter.InitializeScopeManager(ctx, make(map[string]any))

[fishonamos]

Basically, you have 2 possibilities:

• manually defining the ScopeManager field of ctx

  ctx := &hinter.HintRunnerContext{
          ScopeManager: *hinter.DefaultNewScopeManager(),
      }

• creating an uninitialized ctx an calling InitializeScopeManager , passing ctx and an empty array of string to initialize the ScopeManager field:

      ctx := &hinter.HintRunnerContext{}
      hinter.InitializeScopeManager(ctx, make(map[string]any))

Thank you @TAdev0 . I used the Uninitialized ctx option which creates an empty HintRunnerContext first, then initializes the ScopeManager using InitializeScopeManager(). The code now compiles.

[TAdev0]

LGTM!

@cicr99 should we merge this like this and open another PR to modify every test to be consistent with the default initialized scope manager?

[TAdev0]

there is a unit test failing after merging the main branch.

This is becasue of UsortVerify test that uses InitializeScopeManager to create a scope with positions_dict . But as we already call InitializeScopeManager in the general test setting, it wont work.

Can you modify the UsortVerify so that it calls ctx.ScopeManager.EnterScope instead?

[har777]

there is a unit test failing after merging the main branch.

This is becasue of UsortVerify test that uses InitializeScopeManager to create a scope with positions_dict . But as we already call InitializeScopeManager in the general test setting, it wont work.

Can you modify the UsortVerify so that it calls ctx.ScopeManager.EnterScope instead?

Shouldn't he just call AssignVariable? Given a default scope exists now?

[TAdev0]

yes AssignVariable is enough, you are right. I'll do a PR to update all tests configuration once this one is merged anyway

[cicr99]

LGTM!

@cicr99 should we merge this like this and open another PR to modify every test to be consistent with the default initialized scope manager?

I was thinking to do the updating of all tests in this pr as well, but it might take some time and effort, specially with all the merges happening at the moment. I think doing it separately would be the best way to go. Once the tests are passing @fishonamos, we can merge. The PR looks good like this!

[fishonamos]

I modified "Usort Verify" to just call Assign Variable. I think we need to do for all others too, such as UsortVerifyMultiplicityAssert.

[TAdev0]

hi @fishonamos , yes i will update all other tests in another PR to make sure we never create unnecessary scopes :)

[TAdev0]

@fishonamos your unit test dont pass. You should use make build and make unit to run tests locally and make sure they pass. you need to check the return value with AssignVariable

[fishonamos]

They are now passing

[TAdev0]

you need to check the return value with AssigneVariable

err = ctx.ScopeManager.AssignVariable("...", ...)
 if err != nil {
       return err
 }

[fishonamos]

All tests have passed.

[TAdev0]

lgtm!

[TAdev0]

well we should probably have used

        if err != nil {
                        t.Fatal(err)
                    }

in unit tests, but anyway it doesnt matter, and i'm pushing soon a pr that will update this