Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate Set is not modified during iteration #297

Open
pwrightcertinia opened this issue Nov 18, 2024 · 0 comments
Open

Validate Set is not modified during iteration #297

pwrightcertinia opened this issue Nov 18, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@pwrightcertinia
Copy link
Contributor

pwrightcertinia commented Nov 18, 2024

See this new exception, added in API 62: When iterating a set, you are not permitted to call add()/remove() on it, a System.FinalException is thrown.

Given this test (example copied from docs) added to cst/ForTest.scala:

test("for-each modify while iterating") {
    typeDeclaration(
      "public class Dummy {{ Set<String> set_string = new Set<String>{'one', 'two', 'three'}; for(String str : set_string) { set_string.remove(str); } }}"
    )
    assert(
      dummyIssues == "" // error TBD
    )
  }

The generated CST for the loop looks like this:

ForStatement(
  EnhancedForControl(
    String,
    Id(str),
    PrimaryExpression(
      IdPrimary(
        Id(set_string)
      )
    )
  ),
  StatementBlock(
    ArraySeq(
      ExpressionStatement(
        DotExpressionWithMethod(
          PrimaryExpression(
            IdPrimary(
              Id(set_string)
            )
          ),
          false,
          Some(
            MethodCallWithId(
              Id(remove),
              ArraySeq(
                PrimaryExpression(
                  IdPrimary(
                    Id(str)
                  )
                )
              )
            )
          )
        )
      )
    )
  )
)

We can get the Expression / Iteration Type, by breaking down the existing ForControl.verify with a verifyExpression. This can return an ExprContext to later pass to verify (works for both types of ForControl). Then EnhancedForControl.getIterationType can probably be used within ForStatement as well with pattern matching on the control type. Ultimately it's do-able various ways.

So we can know that it is a Set being iterated, and know its ID set_string. The problem comes when trying to then validate the block for calls to set_string.add etc.

A few possible solutions:

  • Add tracking for all method calls to VerifyContext (same as vars) so we can do a lookup on the loopContext after verify.
    • Potentially disable this tracking for performance unless it is needed on a certain context (like this).
    • Or a plugin-like callback/partially applied function w iteration id/type, added to the context, called by the method cst if available.
  • Add the loop info to VerifyContext and check on DotExpressionWithMethod - this check would happen even if we were outside a loop, not great.
  • Add Visitable trait (accept/visit) to CST and use it to search the tree and pattern match the relevant calls.
  • Modify the expression Set type to a readonly variant (Set_ReadOnly,Set<T,A>) so when it is used in later verify calls can be flagged as not allowed.

One final limitation: the error might cross method call boundary, if the set is passed as a param somewhere.

So TL;DR: We need to implement another pattern to make for loops support this kind of validation.

@pwrightcertinia pwrightcertinia added the enhancement New feature or request label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant