florianschanda / miss_hit

MATLAB Independent, Small & Safe, High Integrity Tools - code formatter and more
GNU General Public License v3.0
160 stars 21 forks source link

don't produce style issues if parse errors are present #199

Closed EmilyGraceSeville7cf closed 3 years ago

EmilyGraceSeville7cf commented 3 years ago

MISS_HIT Component affected Please choose one from:

Your operating system and Python version

Describe the bug When I run mh_style test.m for:

clear all;
N = 4;

for i = 1:N
    for j = 1:N
        disp(i + j);
    end
end

it outputs only this:

In test.m, line 1
| clear all;
| ^^^^^ style: file does not appear to contain any copyright header
MISS_HIT Style Summary: 1 file(s) analysed, 1 style issue(s)

But when I run the same command for:

clear all;

function z = f(x, a)
    n = size(a);
    sum = 0;

    for i = 1:1:n(1)
        sum = sum + a(i) * x^(i - 1);
    end

    z = sum;
end

n = 4;
x = [1 3 5 7]';
y = [2 2 4 4]';

for i = 1:n
    for j = 1:n
        X(i, j) = x(i)^(j - 1);
    end
end

a = X^(-1) * y;
disp(a)

f(2, a)
f(4, a)
f(6, a)

it outputs several indention issues:

In test.m, line 1
| clear all;
| ^^^^^ style: file does not appear to contain any copyright header
In test.m, line 14
| n = 4;
| ^ error: expected end of file, found IDENTIFIER instead
In test.m, line 19
|     for j = 1:n
|     ^^^ style: indentation not correct, should be 0 spaces, not 4
In test.m, line 20
|         X(i, j) = x(i)^(j - 1);
|         ^ style: indentation not correct, should be 0 spaces, not 8
In test.m, line 21
|     end
|     ^^^ style: indentation not correct, should be 0 spaces, not 4
MISS_HIT Style Summary: 1 file(s) analysed, 4 style issue(s), 1 error(s)

It is confusing not only because these issues are not shown persistently but also indention rules are different from other language ones like in C#.

florianschanda commented 3 years ago

So the problem with the second file is basically the same that you encountered in #198.

When MISS_HIT encounters a parse error then weird things start to happen. The indenter in particular will get super confused; I should maybe disable any style checking if a parse error is encountered. So these messages are just not helpful.

If you fix the syntax error by moving the function at the end, i.e:

clear all;

n = 4;
x = [1 3 5 7]';
y = [2 2 4 4]';

for i = 1:n
    for j = 1:n
        X(i, j) = x(i)^(j - 1);
    end
end

a = X^(-1) * y;
disp(a)

f(2, a)
f(4, a)
f(6, a)

function z = f(x, a)
    n = size(a);
    sum = 0;

    for i = 1:1:n(1)
        sum = sum + a(i) * x^(i - 1);
    end

    z = sum;
end

Then you'll get this output:

| clear all;
| ^^^^^ style: file does not appear to contain any copyright header
In potato.m, line 14
| disp(a)
|       ^ style: end statement with a semicolon
In potato.m, line 16
| f(2, a)
|       ^ style: end statement with a semicolon
In potato.m, line 17
| f(4, a)
|       ^ style: end statement with a semicolon
In potato.m, line 18
| f(6, a)
|       ^ style: end statement with a semicolon
In potato.m, line 20
| function z = f(x, a)
|              ^ style: violates naming scheme for function

Which makes a lot more sense (i.e. it's happy with the indentation)

florianschanda commented 3 years ago

I'll use this ticket to remove the confusing style messages if a parse error is encountered.

Thank you for bringing this to my attention!

florianschanda commented 3 years ago

@alvinseville7cf this is fixed on master and will be in the next release. For your original code you now just get:

In test_1.m, line 1
| % Taken from issue #199 (by alvinseville7cf)
|                                             ^ style: No copyright notice found in header
In test_1.m, line 16
| n = 4;
| ^ error: expected end of file, found IDENTIFIER instead
MISS_HIT Style Summary: 1 file(s) analysed, 1 style issue(s), 1 error(s)

When I implement #198 I will also improve the error message here, because it would be way more helpful to say something like "in-script functions are an Octave feature and are not valid MATLAB"