Skip to content

Conversation

@Luni-4
Copy link
Collaborator

@Luni-4 Luni-4 commented Apr 2, 2020

NOM metric counts the number of functions and closures contained in a file/trait/class

Thanks in advance for your review! :)

@Luni-4 Luni-4 changed the title Implement NOM Implement NOM metric Apr 2, 2020
@Luni-4
Copy link
Collaborator Author

Luni-4 commented Apr 2, 2020

How can we consider the definition and implementation of a method? Should I count one if I declare and implement the same foo method in the same file or two?

@calixteman
Copy link
Collaborator

How can we consider definition and implementation? Should I count one if I declare and implement the same foo method in the same file or two?

I think we should only consider raw count of definitions.

@Luni-4
Copy link
Collaborator Author

Luni-4 commented Apr 2, 2020

How can we consider definition and implementation? Should I count one if I declare and implement the same foo method in the same file or two?

I think we should only consider raw count of definitions.

I was thinking of this case:

int foo();

int foo() {
 return 0;
}

rust-code-analysis dump:

`- {translation_unit:214} from (1, 1) to (6, 1)
   |- {declaration:236} from (1, 1) to (1, 11) : int foo();
   |  |- {primitive_type:61} from (1, 1) to (1, 4) : int
   |  |- {function_declarator:254} from (1, 5) to (1, 10) : foo()
   |  |  |- {identifier:1} from (1, 5) to (1, 8) : foo
   |  |  `- {parameter_list:277} from (1, 8) to (1, 10) : ()
   |  |     |- {(:17} from (1, 8) to (1, 9) : (
   |  |     `- {):8} from (1, 9) to (1, 10) : )
   |  `- {;:39} from (1, 10) to (1, 11) : ;
   `- {function_definition:235} from (3, 1) to (5, 2)
      |- {primitive_type:61} from (3, 1) to (3, 4) : int
      |- {function_declarator:254} from (3, 5) to (3, 10) : foo()
      |  |- {identifier:1} from (3, 5) to (3, 8) : foo
      |  `- {parameter_list:277} from (3, 8) to (3, 10) : ()
      |     |- {(:17} from (3, 8) to (3, 9) : (
      |     `- {):8} from (3, 9) to (3, 10) : )
      `- {compound_statement:263} from (3, 11) to (5, 2)
         |- {{:43} from (3, 11) to (3, 12) : {
         |- {return_statement:287} from (4, 2) to (4, 11) : return 0;
         |  |- {return:75} from (4, 2) to (4, 8) : return
         |  |- {number_literal:95} from (4, 9) to (4, 10) : 0
         |  `- {;:39} from (4, 10) to (4, 11) : ;
         `- {}:44} from (5, 1) to (5, 2) : }

We have declaration and function definition. Which one should I count? If we count both of them we would have two which is wrong, otherwise one.

But if we define this code and we have decided to count declaration it would be 0.

int foo() {
 return 0;
}

We could count function_declarator to solve the case above, but the problems remains for the first case, we would have 2 which is wrong.

These examples are to explain why I've used strings :)

@calixteman
Copy link
Collaborator

I think we should just count definition (i.e. function_definition).

@Luni-4 Luni-4 requested a review from calixteman April 2, 2020 13:33
@Luni-4 Luni-4 force-pushed the implement-nom branch 2 times, most recently from c714899 to 8b03c13 Compare April 2, 2020 14:22
@Luni-4 Luni-4 requested a review from calixteman April 6, 2020 09:53
@codecov-io
Copy link

codecov-io commented Apr 6, 2020

Codecov Report

Merging #134 into master will increase coverage by 1.87%.
The diff coverage is 37.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   12.92%   14.80%   +1.87%     
==========================================
  Files          44       45       +1     
  Lines        5670     5768      +98     
  Branches      659      684      +25     
==========================================
+ Hits          733      854     +121     
+ Misses       4690     4631      -59     
- Partials      247      283      +36     
Impacted Files Coverage Δ
src/macros.rs 44.44% <ø> (ø)
src/ts_parser.rs 17.94% <ø> (ø)
src/web/server.rs 57.10% <ø> (ø)
src/metrics.rs 27.70% <16.66%> (+<0.01%) ⬆️
src/nom.rs 42.50% <42.50%> (ø)
src/language_rust.rs 0.00% <0.00%> (ø)
src/cyclomatic.rs 38.59% <0.00%> (+10.52%) ⬆️
src/halstead.rs 28.57% <0.00%> (+12.24%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39e4eb2...54110c0. Read the comment docs.

@Luni-4 Luni-4 removed the request for review from calixteman April 6, 2020 13:11
@Luni-4 Luni-4 force-pushed the implement-nom branch 2 times, most recently from bd5f3f2 to 54110c0 Compare April 6, 2020 13:54
@Luni-4 Luni-4 requested a review from calixteman April 6, 2020 14:24
@Luni-4 Luni-4 requested a review from calixteman April 7, 2020 13:01
@calixteman
Copy link
Collaborator

Thx

@calixteman calixteman merged commit c320eac into mozilla:master Apr 7, 2020
@Luni-4 Luni-4 deleted the implement-nom branch April 7, 2020 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants