Ops check for Halstead

[dburriss]

I am creating this branch from #811 to implement the ops check mentioned by @Luni-4

It is not clear to me that this needs to be tied to the Halstead PR for merge so I wanted a PR to discuss and experiment (and ask questions).

[dburriss]

Initially, I was getting this error in the test:

thread 'ops::tests::java_ops' panicked at 'assertion failed: `(left == right)`
  left: `["()", ")", "+", ",", ".", "/", ";", "=", "[]", "]", "integral_type", "method_invocation", "type_identifier", "void_type", "{}", "}"]`,
 right: `["()", "+", ",", "/", ";", "=", "String", "[]", "format", "int", "void", "{}"]`', src\ops.rs:280:9

Tracing through how it works I started playing with is_primitive, with current changes.

thread 'ops::tests::java_ops' panicked at 'assertion failed: `(left == right)`
  left: `["()", ")", "+", ",", ".", "/", ";", "=", "[]", "]", "{}", "}"]`,
 right: `["()", "+", ",", "/", ";", "=", "String", "[]", "format", "int", "void", "{}"]`', src\ops.rs:280:9

Since I have not touched this for a while I am going to have to dig a bit. Expect questions @Luni-4 :D

[Luni-4]

@dburriss

Is there any update on this one?

[dburriss]

@Luni-4 nothing. I have not been able to spend time on this recently. I will try iron out the bugs in the complexity PR I still have open first and then circle around to this.

[Luni-4]

@dburriss

Now that we have tree-sitter-java 0.20 we can try to verify whether this PR works as expected

[dburriss]

@Luni-4 I will update and see what the result is. Will let you know.

[Luni-4]

Any update on this one @dburriss?

[dburriss]

Hey @Luni-4 I checked and same issue. I will try dive in a little this weekend although I wont have too much time so will likely go into the following week.

[Luni-4]

Hey @Luni-4 I checked and same issue. I will try dive in a little this weekend although I wont have too much time so will likely go into the following week.

Perfect! Let me know if there are some problems!

[dburriss]

Hi @Luni-4 A few questions:

1. Do you know why the list for Java is showing the grammar type instead of the value i.e. 'integral_type instead of int as it does for the other languages?
2. Does the order of operators/operands matter in the assert?
3. If you have any thoughts on why the closing braces are showing up as well as the pairs?

 left: `["()", ")", "+", ",", ".", "/", ";", "=", "[]", "]", "integral_type", "method_invocation", "type_identifier", "void_type", "{}", "}"]`,
 right: `["()", "+", ",", "/", ";", "=", "String", "[]", "format", "int", "void", "{}"]`'

Any threads to pull on would be appreciated. I don`t have a stack of time at the moment so I keep having to stop before I have dug in much. Hopefully, this week will be a bit more relaxed and I can dig into this.

[Luni-4]

@dburriss

I'm going to try to reply to your questions:

1. Due to this function https://github.com/mozilla/rust-code-analysis/blob/master/src/getter.rs#L8, we convert operators into String as defined by the grammar enumerator. Here an example for the MethodInvocation. To solve this problem, we need to choose the correct operators (and also operands) for Java in order to have the correct result.
2. It does not count because we sort them out here
3. About the braces, we are using the same trick of point 1 as shown here

I hope I replied to all your questions exhaustively :)

[dburriss]

Hi @Luni-4 Could you validate something for me? So I implemented get_operator_id_as_str as a test and indeed this then returns some of the correct values then but this strikes me as pointless since now I am just making the test pass and not really implementing anything useful as far as I can tell since these mappings are already returning HalsteadType::Operator in get_op_type. If this isn't picking up the correct values from get_op_type i don't see how this is useful. The fact that the other languages do though makes me think that this test relies on some other trait implementation that i don't have for Java. I thought maybe get_func_space_name but overriding that did nothing for the test.

I am a bit stuck here.

[Luni-4]

Hi @dburriss

I think we should use Int which is immediately converted as "int" and add only VoidType as "void" to get_operator_id_as_str. In this way we would get the right results,

I perfectly agree with you, operators and operands should be the same used for Halstead, so we need to write our own operators and operands tests in order to be coherent with our decisions. The only difference it's how we show these operators and operands (VoidType as "void" instead of "void_type" is an example)

[dburriss]

@Luni-4 Some of these changes don't make sense to me (like removing right brackets) but it seems to match other langs and makes the test pass. Let me know.

Ill remove some of the commented-out bits if the changes do indeed make sense.

[dburriss]

@Luni-4 @marco-c just a ping on this as its been open a while.

[marco-c]

@Luni-4 would you mind finishing up the review here?

[Luni-4]

Sorry for the late reply @dburriss, but I have been busy lately.

Please squash commits and clean up commented code.

To be coherent with the other languages we can leave this PR as-is, otherwise we can change how brackets are managed in the other languages. Why doesn't it make sense to you?