42School / norminette

Official 42 norminette
MIT License
973 stars 141 forks source link

If..Else Inside A While Scope #419

Open yumamur opened 1 year ago

yumamur commented 1 year ago

When an if statement exists in a while scope, which is not delimited by curly brackets, Norminette expects the else of the if to be ordinated with the while.

Erroneous code

int main(void)
{
    while (1)
        if (1)
            break ;
    else
        return (0);
    while (1)
        if (1)
            break ;
        else
            return (0);
}
kiokuless commented 1 year ago

Hi @yumamur! You should check the following link. This is not a bug, an intended behavior. If you use an if statement in a while statement, it is obvious that the while statement contains more than two lines. https://github.com/42School/norminette/blob/af5bf146cf4bb6e02fc3469cd1ce12b34c04c65a/pdf/en.norm.tex#L219-L220

yumamur commented 1 year ago

Hi @kiokuless. Thanks for the addition, I reported the error unaware of this fact. But then, even if that is the case, norminette should report the while structure containing more than one line, right? And we can do something like this, too:

// ...
while (1)
    if (1)
        while (1)
            if (1)
                return (1);
// ...
kiokuless commented 1 year ago

Yes, your opinion is better.

But then, even if that is the case, norminette should report the while structure containing more than one line, right?

By the way, I guess it looks like a lot of work to implement this with the current norminette. Because the current norminette implementation is looking at the token sequence by lexical analysis, and doesn't do syntactic analysis. So it seems to be difficult to detect this...

NiumXp commented 1 year ago

I guess it looks like a lot of work to implement this with the current norminette.

I believe we can solve this problem just by viewing the history, if one statement (flow control) is inside another one, we show the error. But, history is extremely limited to do this, we just store strings in history instead of Rule instances.

Example:

class CheckStatements(Rule):
    # New design
    def __init__(self, context, scope):
        self.context = context
        self.scope = scope

    def run(self, context):  # Context to compatibility
        if not self.is_statement(context):
            return
        if last := context.history.last:
            if last.scope != self.scope:
                context.new_error("MORE_THAN_1_LINE", context.peek_token(0))

    def is_statement(self, context):
        ...
kiokuless commented 1 year ago

I came up with a simpler implementation. It is to check the first token of the statement part of if/if-else/while statements.

Since the control structures allowed in norminette are only if/if-else/while, this approach looks good to me. Is there anything that could be left out?

NiumXp commented 1 year ago

@kiokuless, I believe that the problem cannot be solved with lexical analysis (checking if the first token after the conditional of an if, while, etc.) because it is necessary to look where the statement is and not what is in it. So some syntactic analysis would help here: if it's in a BlockStatement then ok, otherwise error.

With your solution, how do you plan to allow the first if and disallow the second in the code below?

void main(void)
{
        if (1)
            return 1;
        if (1)
            if (2)
                return 2;
}