miss_hit icon indicating copy to clipboard operation
miss_hit copied to clipboard

`mh_style --fix` breaking a valid (but oddly-formatted) switch statement

Open j-tyr opened this issue 4 months ago • 0 comments

MISS_HIT Component affected

  • Style checker

Your MATLAB/Octave environment

  • MATLAB
  • R2024a

Your operating system and Python version

  • Windows
  • Python 3.12.6

Describe the bug

I'm working on a codebase that includes the fit_ellipse.m file from fit_ellipse. This includes the following block of code:

switch (1)
case (test>0),  status = '';
case (test==0), status = 'Parabola found';  warning( 'fit_ellipse: Did not locate an ellipse' );
case (test<0),  status = 'Hyperbola found'; warning( 'fit_ellipse: Did not locate an ellipse' );
end

which looks a bit odd (to me at least) but produces 0 warnings under "default" MATLAB settings.

When running mh_style --fix on this file, we get:

switch 1
    case test > 0 status = '';
    case test == 0 status = 'Parabola found';
        warning('fit_ellipse: Did not locate an ellipse');
    case test < 0 status = 'Hyperbola found';
        warning('fit_ellipse: Did not locate an ellipse');
end

which is not valid MATLAB syntax (we get the error "use a newline, semicolon, or comma before this statement" for each status = ... statement).

There's a few things going on at once here in the original code that mh_style is attempting to fix - redundant brackets, and a combination of both commas , and semicolons ; being used to separate statements in a single line. It looks like we're losing the , when we need a newline?

A second run of mh_style --fix actually corrects this:

switch 1
    case test > 0
        status = '';
    case test == 0
        status = 'Parabola found';
        warning('fit_ellipse: Did not locate an ellipse');
    case test < 0
        status = 'Hyperbola found';
        warning('fit_ellipse: Did not locate an ellipse');
end

with a message like:

In third-party/file-exchange/3215-fit_ellipse/fit_ellipse.m, line 191
|         case test > 0 status = '';
|                     ^ style: end statement with a newline [fixed] [end_of_statements]

Is correcting invalid MATLAB syntax within the remit of a formatting tool? I was assuming the second run would just highlight the error in the logs without touching the code.

There's a similar example in this code with an if statement and mixed use of , and ;, where a first run of mh_style --fix introduces a warning that is then fixed in a second run.

Original code:

% make sure coefficients are positive as required
if (a<0), [a,c,d,e] = deal( -a,-c,-d,-e ); end

After one run of mh_style --fix:

% make sure coefficients are positive as required
if a < 0 [a, c, d, e] = deal(-a, -c, -d, -e);
end

After two runs of mh_style --fix:

% make sure coefficients are positive as required
if a < 0
        [a, c, d, e] = deal(-a, -c, -d, -e);
end

j-tyr avatar Oct 07 '24 10:10 j-tyr