Improve AST functionality with excludeMain()

[sarpsahinalp]

Improved the current AST functionality to allow instructors to exclude the Main method from AST checks.

• Added a method, excludeMain(), to avoid AST checking of the Main method, as it is removed from the AST.

[MaisiKoleni]

Why exclude main? Could you describe a use case here? Why not any other method? Or just just specific issues?

This potentially weakens security (or maybe better called reliability, accuracy here). What are the thoughts on that and the rationale?

[Strohgelaender]

Why exclude main? Could you describe a use case here?

I believe the idea here is to allow students restriction-free local testing of their own solution using the main method.

But I'm not sure if just excluding the main method is the best approach for this. Consider a student submission like this:

public static void main(String[] args) {
   testCase1();
   testCase2();
}

Even though the testCasex() methods are clearly only for local testing and have nothing to do with the exercise, they would be included in the analysis. We probably need to implement some kind of dependency tree to distinguish methods (only) used by the entry point(s) and methods used in the solution of the exercise.

Edit: I just noticed that your other PR implements a MethodCallGraph class. I did not find time to look at this PR in detail, but maybe you can combine these approaches.

[MaisiKoleni]

We modify the AST here, it seems. Are we absolutely sure this has no unintended side effects?

[MaisiKoleni]

Or put differently: what does the removal do? Both according to the documentation and the actual implementation?

[sarpsahinalp]

Removal removes the node from the tree and it's child nodes. This seems to be working as intented in the documentation and the actual implementation.

[MaisiKoleni]

If we are going in that direction, a boolean flag is not a sustainable solution (this is quite common).
Consider a design that is more flexible and sustainable. We can still offer a sensible selection of implementations in some way. Think of collect and Collectors, Charset and StandardCharsets, and so on, or similar APIs.

[sarpsahinalp]

If we are going in that direction, a boolean flag is not a sustainable solution (this is quite common). Consider a design that is more flexible and sustainable. We can still offer a sensible selection of implementations in some way. Think of collect and Collectors, Charset and StandardCharsets, and so on, or similar APIs.

I will take a look in to those APIs thank you for your review!

[sarpsahinalp]

Why exclude main? Could you describe a use case here?

I believe the idea here is to allow students restriction-free local testing of their own solution using the main method.

But I'm not sure if just excluding the main method is the best approach for this. Consider a student submission like this:

public static void main(String[] args) {
   testCase1();
   testCase2();
}

Even though the testCasex() methods are clearly only for local testing and have nothing to do with the exercise, they would be included in the analysis. We probably need to implement some kind of dependency tree to distinguish methods (only) used by the entry point(s) and methods used in the solution of the exercise.

Edit: I just noticed that your other PR implements a MethodCallGraph class. I did not find time to look at this PR in detail, but maybe you can combine these approaches.

For this we need to use the MethodCallGraph which is again not in scope of this PR. Will take this to Markus further!