uqbar-project / wollok

Wollok Programming Language
GNU General Public License v3.0
60 stars 16 forks source link

Validation: Non boolean operands on boolean expressions or if #774

Closed javierfernandes closed 8 years ago

javierfernandes commented 8 years ago

This are typically typing errors. There are some grammar elements which require a boolean expression like OR and AND are binary operators which eventually need that both of their operands (left and right side) be a boolean or boolean producing expressions.

Without a type system and type inference we cannot statically check this, but we can check some basic cases like the followings: (this applies to the condition part of the if, but also to and, or and not operations)

/* XPECT_SETUP org.uqbar.project.wollok.tests.xpect.WollokXPectTest END_SETUP */

object pepita {
}

object p {
    method run() {
        // XPECT errors --> "Non boolean expression" at "null"
        if (null)
            throw new Exception("asd")

        // XPECT errors --> "Non boolean expression" at "2"
        if (2)
            throw new Exception("asd")

        // XPECT errors --> "Non boolean expression" at "2+2"
        if (2+2)
            throw new Exception("asd")

        // XPECT errors --> "Non boolean expression" at "2" 
        a = 2 && p != null

        // XPECT errors --> "Non boolean expression" at "2" 
        a = 2 || p != null

        // XPECT errors --> "Non boolean expression" at "p" 
        a = pepita || p != null

        // XPECT errors --> "Non boolean expression" at "new List()"    
        a = new List() || p != null

        // more cases with method call require a type system with inference
    }
}
javierfernandes commented 8 years ago

Another one that is already validated in runtime by 61aefa8

var expresionNula
if (expresionNula)
npasserini commented 8 years ago

There are some issues that we might allow as warnings. For example I sometimes modify my code from

if (some condition) { blah }

temporarily to

if (true || some condition) { blah } just for debugging purposes.. what do you think?

On Tue, Jun 28, 2016 at 2:46 PM, javierfernandes notifications@github.com wrote:

Another one that is already validated in runtime by 61aefa8 https://github.com/uqbar-project/wollok/commit/61aefa8a60d5217d54882e9a705892df063410cd

var expresionNulaif (expresionNula)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/uqbar-project/wollok/issues/774#issuecomment-229038355, or mute the thread https://github.com/notifications/unsubscribe/AEa1OWr3h6Y_aKedAZnB-q1stva2UMr1ks5qQResgaJpZM4I_qMH .

javierfernandes commented 8 years ago

Yes could be a warning.

matifreyre commented 8 years ago

Will it be configurable? I'd like those as errors for students by default, but also that we can change them for warnings for these cases.

On Tue, Jun 28, 2016 at 11:57 AM, javierfernandes notifications@github.com wrote:

Yes could be a warning.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/uqbar-project/wollok/issues/774#issuecomment-229075562, or mute the thread https://github.com/notifications/unsubscribe/AJ-EZiGVMdHAI0Y8Xa4ImMEgW2GHVj8Eks5qQTZjgaJpZM4I_qMH .

npasserini commented 8 years ago

Every validation should be configurable but in a global base, I do not know if that is what you mean.

On Tue, Jun 28, 2016 at 4:59 PM, Matías Freyre notifications@github.com wrote:

Will it be configurable? I'd like those as errors for students by default, but also that we can change them for warnings for these cases.

On Tue, Jun 28, 2016 at 11:57 AM, javierfernandes < notifications@github.com> wrote:

Yes could be a warning.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://github.com/uqbar-project/wollok/issues/774#issuecomment-229075562 , or mute the thread < https://github.com/notifications/unsubscribe/AJ-EZiGVMdHAI0Y8Xa4ImMEgW2GHVj8Eks5qQTZjgaJpZM4I_qMH

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/uqbar-project/wollok/issues/774#issuecomment-229076221, or mute the thread https://github.com/notifications/unsubscribe/AEa1OenOnls9cNE2byIFhthJZ02qx3cCks5qQTbWgaJpZM4I_qMH .

matifreyre commented 8 years ago

Yes, that's it

On Tue, Jun 28, 2016 at 7:49 PM, Nico Passerini notifications@github.com wrote:

Every validation should be configurable but in a global base, I do not know if that is what you mean.

On Tue, Jun 28, 2016 at 4:59 PM, Matías Freyre notifications@github.com wrote:

Will it be configurable? I'd like those as errors for students by default, but also that we can change them for warnings for these cases.

On Tue, Jun 28, 2016 at 11:57 AM, javierfernandes < notifications@github.com> wrote:

Yes could be a warning.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <

https://github.com/uqbar-project/wollok/issues/774#issuecomment-229075562

, or mute the thread <

https://github.com/notifications/unsubscribe/AJ-EZiGVMdHAI0Y8Xa4ImMEgW2GHVj8Eks5qQTZjgaJpZM4I_qMH

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/uqbar-project/wollok/issues/774#issuecomment-229076221 , or mute the thread < https://github.com/notifications/unsubscribe/AEa1OenOnls9cNE2byIFhthJZ02qx3cCks5qQTbWgaJpZM4I_qMH

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/uqbar-project/wollok/issues/774#issuecomment-229207645, or mute the thread https://github.com/notifications/unsubscribe/AJ-EZu-yjjLw6qNFx15SATztHArsjNM3ks5qQaTzgaJpZM4I_qMH .

javierfernandes commented 8 years ago

Yes. They are global or per project.

What we dont have yet is an export import of theconf to a file or something

El Tuesday, June 28, 2016, Matías Freyre notifications@github.com escribió:

Yes, that's it

On Tue, Jun 28, 2016 at 7:49 PM, Nico Passerini <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Every validation should be configurable but in a global base, I do not know if that is what you mean.

On Tue, Jun 28, 2016 at 4:59 PM, Matías Freyre <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Will it be configurable? I'd like those as errors for students by default, but also that we can change them for warnings for these cases.

On Tue, Jun 28, 2016 at 11:57 AM, javierfernandes < notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Yes could be a warning.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <

https://github.com/uqbar-project/wollok/issues/774#issuecomment-229075562

, or mute the thread <

https://github.com/notifications/unsubscribe/AJ-EZiGVMdHAI0Y8Xa4ImMEgW2GHVj8Eks5qQTZjgaJpZM4I_qMH

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <

https://github.com/uqbar-project/wollok/issues/774#issuecomment-229076221

, or mute the thread <

https://github.com/notifications/unsubscribe/AEa1OenOnls9cNE2byIFhthJZ02qx3cCks5qQTbWgaJpZM4I_qMH

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/uqbar-project/wollok/issues/774#issuecomment-229207645 , or mute the thread < https://github.com/notifications/unsubscribe/AJ-EZu-yjjLw6qNFx15SATztHArsjNM3ks5qQaTzgaJpZM4I_qMH

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/uqbar-project/wollok/issues/774#issuecomment-229207952, or mute the thread https://github.com/notifications/unsubscribe/AEORWLo1ikNm870ORsDfZ8_tPRFn2Vd9ks5qQaVmgaJpZM4I_qMH .

javierfernandes commented 8 years ago

I've splitted this issue into two, this one and #803

This one is for non-boolean values both in if condition and in boolean expressions.

I know that the and/or can be overloaded (overriden) by customs classes and without a type system they can change the semantic of this operations and therefore render this validator useless or non-appropriated, but I think that those should be the less cases.

I'm thinking on introducing a code annotation to bypass checks in such a cases. Like many other tools have

// > ignore (RedundantBooleanValue)
method myMethod() {
     return true || self.whatever()
}

This comment-level metadata could be as part of method comment, or a class-level. If possible could also be the comment just on top of the offending line.

I'll write a different issue for this.