Add FieldDeclarationVisitor

[LoiNguyenCS]

No description provided.

[LoiNguyenCS]

Professor,

This is the PR to add the visitor that will tell us all declared fields in a Java file. Please take a look. Thank you.

[kelloggm]

Overall looks very good, but I have one concern about the difference between local variable accesses and field accesses, since both are syntactically the same but the code doesn't seem to account for the possibility that a NameExpr might be a local variable.

[LoiNguyenCS]

Professor @kelloggm,

Overall looks very good, but I have one concern about the difference between local variable accesses and field accesses, since both are syntactically the same but the code doesn't seem to account for the possibility that a NameExpr might be a local variable.

Thank you for bringing this to my attention. I have forgotten that unsolved NameExpr could also include unsolved local variables. I think the problem will now get more complicated since we have to differentiate between unsolved fields of the current class, unsolved local variables, and unsolved fields of superclass.

I will see what I can do to solve this problem.

[kelloggm]

unsolved local variables

Fortunately, local variables are just that - local. So, it should be relatively easy to check what is in-scope.

[LoiNguyenCS]

Fortunately, local variables are just that - local. So, it should be relatively easy to check what is in-scope.

Thanks for telling me. I was about to upgrade FieldDeclarationsVisitor to make sure there are no unsolved fields left before passing the input file to UnsolvedSymbolVisitor. But your approach sounds much more doable 💯

[LoiNguyenCS]

I was looking for methods to find the scope of a variable using JavaParser when I found this issue. It seems to be that to know the scope of a variable, we need to trace back its declaration, which would require the variable to be solvable.

I've also tried getting the parentNode of the NameExpr, but that doesn't work either. For example, no matter if shot is an unsolved local variable, field of the current class, or field from the superclass, its parentNode will be return shot;. If we keep tracing back, no matter what shot is, we will end up with the method declaration of takeAShot, which is not useful at all.
`

public int takeAShot() {
...
return shot;
}

[kelloggm]

I was looking for methods to find the scope of a variable using JavaParser when I found this javaparser/javaparser#2673. It seems to be that to know the scope of a variable, we need to trace back its declaration, which would require the variable to be solvable.

There does seem to be some kind of getDeclaration() API. Using that, we could probably check whether something is a local variable declaration.

An alternative would just be to keep a local variable scope table ourselves, similar to what is mentioned in this comment: effectively, we add an entry to this table every time we visit a local variable declaration, and we clear the table every time we exit a scope block. This might require us to be a bit careful to manage nested blocks, but I think it would be relatively easy for us to implement and we should do so.

[LoiNguyenCS]

Professor,

I would like to request more time to address this issue, which appears to be a much more complex than originally anticipated. Several difficulties have arisen.

The first challenge is the getDeclaration function, which only works for solvable symbols.

The second complication arises from our use of JavaParser, which can not inform us when we exit a specific scope in the code. So to keep track of those local variables, we need to build a Map to note down every local variable and its scope by visiting all the VariableDeclarator instances beforehand.

The third and perhaps the trickiest part involves handling special local variables, such as those within nested if-else statements. For a local variable within a method, we can conveniently set the key in a Map (let's refer to it as Map A) as the variable's name, with the value being the method's name. However, the scenario becomes different when dealing with constructs like if-else, while, try-catch blocks, and the like. Determining what the key's value should be in such cases is not straightforward.

I've been experimenting with a potential solution where the value of a key in Map A represents the span of lines. For example, if method B has a local variable 'c,' the key in Map A would be 'c,' and its value would be denoted as 'x-y,' where 'x' is the starting line of the method B's declaration, and 'y' is the ending line. This approach, while promising, is quite tiring for me. It requires writing specific code for each special case, such as if-else statements, while loops, try-catch blocks, etc. Working with line numbers also adds complexity and demands careful handling.

When I got tired of dealing with the first solution, I have also tried a second one. In this second solution, I attempted to use the UnsolvedSymbolVisitor to resolve every local variable before dealing with any super variables. My hypothesis is based on the fact that the runtime of the UnsolvedSymbolVisitor is determined by the longest unsolved call statement in the input code. For example, if the longest unsolved call statement in the input code is 'a.b.c.d.e.x()', then UnsolvedSymbolVisitor will be called 6 times.. If we were to run the UnsolvedSymbolVisitor a number of times equivalent to the length of the longest unsolved call statement, then every local variable should, theoretically, be resolved. While this approach appears to work for my test cases, I cannot definitively prove its correctness.

[LoiNguyenCS]

Professor, I have added a simple symbol table to Specimin. Please take a look. Thank you.

[kelloggm]

I think there are some scoping constructs that can contain local variables that this PR doesn't yet handle, especially BlockStmts. You should go over the whole list of Java Parser Nodes and make sure that there are not others, too.

Further, I think the logic here is broken in the case of anonymous classes. Please add a test with an anonymous class inside a method to be sure.

[LoiNguyenCS]

Professor, I have updated the program for it to work with anonymous classes. Please take a look. Regarding the warnings of ErrorProne, I will fix them in a separate PR. Thank you.

[kelloggm]

Overall, this PR is in pretty good shape and my comments are relatively minor.

[kelloggm]

Regarding the warnings of ErrorProne, I will fix them in a separate PR.

When you do, please also reconfigure CI so that it fails any time that ErrorProne warns.

[LoiNguyenCS]

Professor,
I have updated the program based on your suggestions.