[INFRA] set up miss_hit linter config and github action

[Remi-Gau]

• set up miss_hit configuration
• set up corresponding github action
• lint files that are not involved in another PR <-- left out for later to prevent merge conflicts headaches.
• set metric threshold (cyclomatic complexity, file length, nesting depth...) to be VERY tolerant for now

For now the rules concerning functions, classes and methods names are ignored.

[gllmflndn]

Sorry, I had missed the review request. One thing I am not sure about is moving the copyright text at the top of the file. None of Brainstorm, EEGLAB, FieldTrip or SPM does it. MATLAB doesn't but Octave does. When in Rome, do as the Romans...
Is this something that the linter expects?

[Remi-Gau]

Is this something that the linter expects?

Yup as far as I know.

https://florianschanda.github.io/miss_hit/style_checker.html

Copyright notice ("copyright_notice")
This rules looks for a copyright notice at the beginning of each file in the form of "(C) Copyright YEAR-YEAR ENTITY" or "(C) Copyright YEAR ENTITY". The list of acceptable entities can be configured with copyright_entity. This option can be given more than once to permit a set of valid copyright holders. If this options is not set, the rule just looks for any copyright notice.
Configuration parameters:

• copyright_entity: Can be specified more than once. Make sure each copyright notice mentions one of these.
• copyright_in_embedded_code: Normally this rule is not enabled on MATLAB code embedded in Simulink® models, since most models carry their copyright notice elsewhere. This flag can be used to turn on this rule for embedded code tool.

• Option 1: I can check the copyright_in_embedded_code option see if this gives us more lattitude.
• Option 2: turn off that rule and do as we please.

Will test option 1 and let you know.

I am OK with option 2: where we put the copyright is something I clearly do not feel strongly about. :-)

[Remi-Gau]

OK so option 1 makes miss_hit complain.

So we can go with options 2 for now and silence that rule for now: will proceed if I get a 👍

[gllmflndn]

👍 for that. I can see that you also add some indentation in some (but not all?) files in the function block. Which configuration option of MISS_HIT enforces this? Again, I don't think any of the software listed above (including MATLAB) do so and it's a waste of space (especially if you want to limit line length).

[Remi-Gau]

I can see that you also add some indentation in some (but not all?) files in the function block. Which configuration option of MISS_HIT enforces this? Again, I don't think any of the software listed above (including MATLAB) do so and it's a waste of space (especially if you want to limit line length).

Actually that one is going to be harder to circumvent as far as I know:

see here

Mandatory rules
These rules are always active. The technical reason for this is that it would be too difficult to autofix issues without autofixing these. If you pay me an excessive amount of money I could look into this but I'd rather keep the lexer vaguely sane. All of them are automatically fixed by mh_style when the --fix option is specified.
Use of tab
This rule enforces the absence of the tabulation character everywhere. When auto-fixing, a tab-width of 4 is used by default, but this can be configured with the options 'tab_width'.
Note that the fix replaces the tab everywhere, including in strings literals.

So the thing we can do is tweak the tab_width to waste less space, but the indentation on every line in functions seems to be a "must have" (it does make the code look more like python I agree).

It is strange at first but I have set matlab to auto-indent stuff that way now, I would say that the headackes that miss_hit saves us on are worth the little "idiosyncrasy".

I can set the tab-width to 2: that might make make thins more crammed but still readable.

[Remi-Gau]

Rebased and set the tab length to 2.

Let me know what you think. :-)

[Remi-Gau]

FYI: at the moment the linter ignore the source code and it is only applied to the test codebase.

If we check the metrics of the rest of the code base that exceed the threshold set in the cfg with mh_metric . --ci this is what we would get.

In brief: a little of refactoring ahead. Wooohooo !! 🚀 🎉

+bids/layout.m: metric: exceeded file_length: measured 673 > limit 600
In +bids/query.m, line 1
| function result = query(BIDS,query,varargin)
|                   ^^^^^ metric: exceeded cnest: measured 8 > limit 6
In +bids/query.m, line 1
| function result = query(BIDS,query,varargin)
|                   ^^^^^ metric: exceeded cyc: measured 43 > limit 20
In +bids/report.m, line 1
| function report(BIDS, Subj, Ses, Run, ReadNII)
|          ^^^^^^ metric: exceeded cyc: measured 31 > limit 20
In +bids/+internal/file_utils.m, line 1
| function varargout = file_utils(str,varargin)
|                      ^^^^^^^^^^ metric: exceeded cyc: measured 21 > limit 20
In +bids/+util/tsvread.m, line 1
| function fileContent = tsvread(filename, fieldToReturn, hdr)
|                        ^^^^^^^ metric: exceeded cyc: measured 25 > limit 20
In +bids/+util/tsvwrite.m, line 1
| function tsvwrite(f, var)
|          ^^^^^^^^ metric: exceeded cyc: measured 22 > limit 20
MISS_HIT Metric Summary: 18 file(s) analysed, 7 metric deviations(s)

[Remi-Gau]

I have reset this and only kept the config file of the linter and the continuous integration to prevent any merge conflicts for now as the linter tends to change almost all the lines of our files for now.

We'll clean things step by step as we go, PR by PR and only for the files changed in a given PR or before the creation of a first release.