Generalize Nested Structure Counting for languages

[rakhimov]

The code is refactored to work with nesting level metric provided by language readers of Lizard. The feature is only tested with Python and C++. This implementation should be enough to close #69.

[rakhimov]

Hi @terryyin, do not yet merge this PR. I sense there are bugs in implementation of C++ current_nesting_level. The initial code works with Python, for which the nesting level metric is producing what is expected. However, the C++ current_nesting_level doesn't seem to be fully conformant.

Your suggestions are welcome.

[terryyin]

Hi @rakhimov Great! Let me merge it first and then find a time to go through it more carefully.

[rakhimov]

Hi @terryyin , I am sorry, but you have terrible PR practices.

[terryyin]

Yeah, the travis CI just said so as well:-)

[terryyin]

However, the C++ current_nesting_level doesn't seem to be fully conferment.

Possible, since it hasn't been used before. Would you fix the failing tests? Is it because of the current_nesting_level not working?

[rakhimov]

Yes, the C++ version is not calculating the expected quantity. It would be great to clarify what the 'nesting_level' is so that other language readers implement it correctly.

I was thinking about fixing the C++ version and submitting another patch which should have been merged first before this PR.

Now it seems things are going to be upside-down.

From the code of CLike reader, the nesting level is increased/decreased on namespace, class, function, whenever '{', '}' appear.

It doesn't detect cases without braces. I am thinking about importing the original logic from 'Nesting Structure count' to get cases without braces.

[terryyin]

It doesn't detect cases without braces. I am thinking about importing the original logic from 'Nesting Structure count' to get cases without braces.

Yes, that’s where the missing piece is. Will you fix that?

I will revert the last merge and wait for this fix.

Regarding “clarify what the ‘nesting_level’ is”, let’s do it when our code works.

On 9 May 2016, at 9:29 AM, Olzhas Rakhimov notifications@github.com wrote:

Yes, the C++ version is not calculating the expected quantity. It would be great to clarify what the 'nesting_level' is so that other language readers implement it correctly.

I was thinking about fixing the C++ version and submitting another patch which should have been merged first before this PR.

Now it seems things are going to be upside-down.

From the code of CLike reader, the nesting level is increased/decreased on namespace, class, function, whenever '{', '}' appear.

It doesn't detect cases without braces. I am thinking about importing the original logic from 'Nesting Structure count' to get cases without braces.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/terryyin/lizard/pull/120#issuecomment-217759177

[terryyin]

It doesn't detect cases without braces.

I meant that’s where the missing piece is.

[rakhimov]

I wish GitHub had unmerge instead of revert. To make it cleaner you can hard reset to the head before this PR merge, so that nothing shows up in history. That's git power.

[terryyin]

Yes, but do you know what will happen to our pull-request history then? I have a revert pull request and a "revert revert" pending pull request.

Hmm, sounds like something I should at least try.

[rakhimov]

It is already mess. GitHub doesn't allow deleting of PRs, so they can only be renamed.

[terryyin]

OK. Now I hard reset the commits.

You don't have to create another pull request for the last change. My "revert revert" pull request seems still work after the reset.

[rakhimov]

The 'revert revert' one will cancel out all your efforts with hard reset because it carries the old history, so all 'reverting' PR will show up again. Just reopen this PR.

[terryyin]

How to reopen the PR? It seems I cannot mark it back to open.

For the 'revert revert' all I need is to do another hard reset after the merge.

[rakhimov]

This PR is technically unmerged, so 'Reopen' button should get activated. Probably GitHub is confused what is going on.

Having phantom PR on the upstream doesn't let me manipulate my own patches. Anyway, this is enough trouble for today. I guess the current state might work.

[terryyin]

Thanks for the guidance:-)

[rakhimov]

Hi @terryyin , if this PR doesn't reopen after closing and deleting the 'revert-revert' PR #122, you can manually merge the updated branch from my repository with the following commands:

git remote add rakhimov https://github.com/rakhimov/lizard
git remote update rakhimov
git merge --no-ff -m "Merge pull request #120 from rakhimov/nested-structures" rakhimov/nested-structures
git remote remove rakhimov

The merged commit should show up in this thread to indicate the relevance.

[terryyin]

@rakhimov done, but I found three failing tests after the merge. Are these expectations wrong now?

======================================================================
FAIL: test_else_if (test.testNestedStructures.TestCppNestedStructures)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/terry/git/lizard/test/testNestedStructures.py", line 52, in test_else_if
    self.assertEqual(1, result[0].max_nested_structures)
AssertionError: 1 != 2

======================================================================
FAIL: test_equal_metric_structures (test.testNestedStructures.TestCppNestedStructures)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/terry/git/lizard/test/testNestedStructures.py", line 123, in test_equal_metric_structures
    self.assertEqual(2, result[0].max_nested_structures)
AssertionError: 2 != 3

======================================================================
FAIL: The last 'else' is associated with the innermost 'if'.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/terry/git/lizard/test/testNestedStructures.py", line 217, in test_gotcha_if_else
    self.assertEqual(3, result[0].max_nested_structures)
AssertionError: 3 != 4

----------------------------------------------------------------------
Ran 475 tests in 0.426s

[rakhimov]

You have to merge #123 first.

[terryyin]

OK. Done. Thanks:-)

[rakhimov]

Great, Thanks!