-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bugfixes and new rules from collaboration EUMETSAT-AQCLab #230
base: master
Are you sure you want to change the base?
Bugfixes and new rules from collaboration EUMETSAT-AQCLab #230
Conversation
…ndexOutOfBoundsException) were produced when analyzed files contained declaration of functions or subroutines with modifiers Recursive, Elemental or Pure
…a file only contains comments, no source code. Also a JFlexException was thrown in the case of a file without code nor comments. The throw for this Exception has been removed to avoid parsing errors in the results.
…n analyzing files that included sentences like "PROCEDURE(proc_interface), POINTER :: proc"
…ld of LOC (set at 1000). Only lines of code are taken into account, comments and blank lines are ignored.
…eshold of LOC (set at 150). Only lines of code are taken into account, comments and blank lines are ignored.
Rule to check the maximum number of arguments in each procedure, which must be at most 7.
Rule to check the maximum value of the cyclomatic complexity in each function, which at most must be 15.
Rule to check that only one module or program is declared in each file.
Rule to check that in each file there are at least 30% of lines of comments with respect to the total number of lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can you provide me some Fortran files that break the new rules and the fixed rules, so that I can test it?
The merge request will be approved after the new rules tested and when my suggestions will be taken into account or discussed with me to clarify.
fortran77-rules/src/main/resources/lex/COMFLOWCheckArguments.lex
Outdated
Show resolved
Hide resolved
fortran77-rules/src/main/resources/lex/COMFLOWCheckArguments.lex
Outdated
Show resolved
Hide resolved
fortran77-rules/src/main/resources/lex/COMMETComplexitySimplified.lex
Outdated
Show resolved
Hide resolved
fortran77-rules/src/main/resources/lex/COMMETComplexitySimplified.lex
Outdated
Show resolved
Hide resolved
fortran90-rules/src/main/resources/lex/COMFLOWCheckArguments.lex
Outdated
Show resolved
Hide resolved
<languageId>fr.cnes.icode.fortran90</languageId> | ||
</check> | ||
<check> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove it if you decide to enhance the existing header rule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pending of confirmation on comment #230 (comment)
<languageId>fr.cnes.icode.fortran77</languageId> | ||
</check> | ||
<check> | ||
<class>fr.cnes.icode.fortran77.rules.F77FILEHeader</class> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove it if you decide to enhance the existing header rule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pending of confirmation on comment #230 (comment)
@@ -109,47 +109,43 @@ STRING = \'[^\']*\' | \"[^\"]*\" | |||
|
|||
private void raiseErrors() throws JFlexException { | |||
LOGGER.finest("begin method raiseErrors"); | |||
if(linesType.isEmpty()){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You decided to remove the fact that we raise an error if file is blank.
Can it be possible to really have blank files in FORTRAN?
If yes, then I will join your choice.
If no, we should implement a rule checking if there is empty files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raising an error in this specific rule and not in others seemed arbitrary, since checking empty files is not the purpose of this rule.
Having a blank file is possible (we found it in one of the projects we evaluated at EUMETSAT). Even though it doesn't present a great problem, such a file would be useless and should be avoided, the same way unused variables should be avoided. So, for this purpose we agree on having a specific rule that checks this situation.
As an alternative, we would favor simply ignoring blank files, but not raise an error in an unrelated rule such as COMPROJECTHeader.
@@ -104,90 +104,86 @@ STRING = \'[^\']*\' | \"[^\"]*\" | |||
} | |||
|
|||
private void raiseErrors() throws JFlexException { | |||
if(this.linesType.isEmpty()){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment than for fortran77 COMPROJECTHeader
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raising an error in this specific rule and not in others seemed arbitrary, since checking empty files is not the purpose of this rule.
Having a blank file is possible (we found it in one of the projects we evaluated at EUMETSAT). Even though it doesn't present a great problem, such a file would be useless and should be avoided, the same way unused variables should be avoided. So, for this purpose we agree on having a specific rule that checks this situation.
As an alternative, we would favor simply ignoring blank files, but not raise an error in an unrelated rule such as COMPROJECTHeader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change these values too to respect RNC
Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
…ied.lex Co-authored-by: Diego Rodriguez <[email protected]>
…ied.lex Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
…ied.lex Co-authored-by: Diego Rodriguez <[email protected]>
…ied.lex Co-authored-by: Diego Rodriguez <[email protected]>
Co-authored-by: Diego Rodriguez <[email protected]>
Proposed changes
Fixing of parsing errors detected in three rules: COM.Inst.Brace, COM.PROJECT.Header, COM.NAME.Homonymy
Implementation of 8 new rules (from EUMETSAT coding standard, but useful in other contexts) to check:
Types of changes
What types of changes does your code introduce to i-Code?
Issues closed by changes
Checklist
Further comments
The changes implemented were sponsored and agreed with EUMETSAT, based on their needs, but both EUMETSAT and AQCLab consider that these may be useful to other parties, and for that may be considered to be included in the offical development of i-Code.