osate / osate2

Open Source AADL2 Tool Environment
http://osate.org
Eclipse Public License 2.0
39 stars 8 forks source link

Incomplete type checking for property constants #2222

Closed lwrage closed 4 years ago

lwrage commented 4 years ago

Summary

There are cases where a property constant of the wrong type can be used without OSATE marking that as an error.

Expected and Current Behavior

The following AADL files should have errors:

property set PS is

    R1: type record (f1: aadlinteger;);
    R2: type record (f1: aadlstring;);
    C1: constant PS::R1 => [f1 => 123;];
    C2: constant PS::R2 => [f1 => "abc";];

    -- should be an error: default value has the wrong type
    C3: constant PS::R1 => PS::C2;

    -- should be an error: default value has the wrong type
    P1: PS::R1 => PS::C2 applies to (all);

    P2: list of PS::R1 applies to (all);

end PS;
package P
public
    with PS;

    abstract A
        properties
            -- should be an error: value has the wrong type
            PS::P1 => PS::C2;
            -- should be an error: list element has the wrong type
            PS::P2 => (PS::C2);
    end A;

end P;

Environment

AaronGreenhouse commented 4 years ago

Looking at the type checking in the validator there are two methods involved here:

  1. PropertiesJavaValidator.typeCheckPropertyValues()
  2. PropertiesJavaValidator.typeMatchRecordFields()

The short answer here is that nothing checks the type of the record against the declared type of the property (or property constant). It only checks that the type of the property is a record type of some sort.

The longer answer is that it then checks the record value itself and recursively checks that the record field values are type correct.

Of course, it is also the case here that record-typed record fields are not type checked correctly either. If we add the following to the above property set, both Z1 and Z2 are error-free even though Z1 is wrong:

    RR1: type record (q1: PS::R1; q2: PS::R2;);

    -- WRONG
    Z1: constant PS::RR1 => [q1 => PS::C2; q2 => PS::C1;];

    -- OKAY
    Z2: constant PS::RR1 => [q1 => PS::C1; q2 => PS::C2;];
AaronGreenhouse commented 4 years ago

Also, there is nothing in the validator that checks that the field names are form the correct record type. This doesn't seem to matter because the cross-reference name binder process seems to take care of that.

AaronGreenhouse commented 4 years ago

Lists of record values are also not checked:

    -- WRONG
    LL1: constant list of PS::RR1 => (PS::C1, PS::C2);
AaronGreenhouse commented 4 years ago

Property reference is also bad:

    P1: PS::R1 => PS::C2 applies to (all);
    -- bad
    PP: PS::R2 => PS::p1 applies to (all);
AaronGreenhouse commented 4 years ago

Okay new improved test case:

Props.aadl

property set Props is
    RecordType1: type record (f1: aadlinteger;);
    RecordType2: type record (f1: aadlstring;);
    C1: constant Props::RecordType1 => [f1 => 123;];
    C2: constant Props::RecordType2 => [f1 => "abc";];

    GoodConstant1: constant Props::RecordType1 => Props::C1; -- good
    GoodConstant2: constant Props::RecordType2 => Props::C2; -- good
    GoodDefault1: Props::RecordType1 => Props::C1 applies to (all);
    GoodDefault2: Props::RecordType2 => Props::C2 applies to (all);
    GoodPA1: Props::RecordType1 applies to (all);
    GoodPA2: Props::RecordType2 applies to (all);

    BadConstant1: constant Props::RecordType1 => Props::C2; -- bad
    BadConstant2: constant Props::RecordType2 => Props::C1; -- bad
    BadDefault1: Props::RecordType1 => Props::C2 applies to (all);
    BadDefault2: Props::RecordType2 => Props::C1 applies to (all);
    BadPA1: Props::RecordType1 applies to (all);
    BadPA2: Props::RecordType2 applies to (all);

    GoodListConstant1: constant list of Props::RecordType1 => (Props::C1, [f1 => 0;]);
    GoodListConstant2: constant list of Props::RecordType2 => (Props::C2, [f1 => "hello";]);
    GoodListDefault1: list of Props::RecordType1 => (Props::C1, [f1 => 0;]) applies to (all);
    GoodListDefault2: list of Props::RecordType2 => (Props::C2, [f1 => "hello";]) applies to (all);
    GoodListPA1: list of Props::RecordType1 applies to (all);
    GoodListPA2: list of Props::RecordType2 applies to (all);

    BadListConstant1: constant list of Props::RecordType1 => (Props::C1, Props::C2, [f1 => 0;]);
    BadListConstant2: constant list of Props::RecordType2 => (Props::C1, Props::C2, [f1 => "hello";]);
    BadListDefault1: list of Props::RecordType1 => (Props::C1, Props::C2, [f1 => 0;]) applies to (all);
    BadListDefault2: list of Props::RecordType2 => (Props::C1, Props::C2, [f1 => "hello";]) applies to (all);
    BadListPA1: list of Props::RecordType1 applies to (all);
    BadListPA2: list of Props::RecordType2 applies to (all);

    RecordOfRecordTypes: type record (q1: Props::RecordType1; q2: Props::RecordType2;);

    GoodFieldsConstant1: constant Props::RecordOfRecordTypes => [q1 => Props::C1; q2 => Props::C2;];
    GoodFieldsListConstant1: constant list of Props::RecordOfRecordTypes => ([q1 => Props::C1; q2 => Props::C2;]);
    GoodFieldsDefault1: Props::RecordOfRecordTypes => [q1 => Props::C1; q2 => Props::C2;] applies to (all);
    GoodFieldsListDefault1: list of Props::RecordOfRecordTypes => ([q1 => Props::C1; q2 => Props::C2;]) applies to (all);
    GoodFieldsPA1: Props::RecordOfRecordTypes applies to (all);
    GoodFieldsListPA: list of Props::RecordOfRecordTypes applies to (all);

    BadFieldsConstant1: constant Props::RecordOfRecordTypes => [q1 => Props::C2; q2 => Props::C1;];
    BadFieldsListConstant1: constant list of Props::RecordOfRecordTypes => ([q1 => Props::C2; q2 => Props::C1;]);
    BadFieldsDefault1: Props::RecordOfRecordTypes => [q1 => Props::C2; q2 => Props::C1;] applies to (all);
    BadFieldsListDefault1: list of Props::RecordOfRecordTypes => ([q1 => Props::C2; q2 => Props::C1;]) applies to (all);
    BadFieldsPA1: Props::RecordOfRecordTypes applies to (all);
    BadFieldsListPA1: list of Props::RecordOfRecordTypes applies to (all);

    R1_prop: Props::RecordType1 applies to (all);
    R2_prop: Props::RecordType2 applies to (all);

    GoodPropRefConstant1: constant Props::RecordType1 => Props::R1_prop;
    GoodPropRefConstant2: constant Props::RecordType2 => Props::R2_prop;
    GoodPropRefDefault1: Props::RecordType1 => Props::R1_prop applies to (all);
    GoodPropRefDefault2: Props::RecordType2 => Props::R2_prop applies to (all);
    GoodPropRefPA1: Props::RecordType1 applies to (all);
    GoodPropRefPA2: Props::RecordType2 applies to (all);

    BadPropRefConstant1: constant Props::RecordType1 => Props::R2_prop;
    BadPropRefConstant2: constant Props::RecordType2 => Props::R1_prop;
    BadPropRefDefault1: Props::RecordType1 => Props::R2_prop applies to (all);
    BadPropRefDefault2: Props::RecordType2 => Props::R1_prop applies to (all);
    BadPropRefPA1: Props::RecordType1 applies to (all);
    BadPropRefPA2: Props::RecordType2 applies to (all);

    GoodListOfPropRefConstant1: constant list of Props::RecordType1 => (Props::R1_prop, [f1 => 0;]);
    GoodListOfPropRefConstant2: constant list of Props::RecordType2 => (Props::R2_prop, [f1 => "hello";]);
    GoodListOfPropRefDefault1: list of Props::RecordType1 => (Props::R1_prop, [f1 => 0;]) applies to (all);
    GoodListOfPropRefDefault2: list of Props::RecordType2 => (Props::R2_prop, [f1 => "hello";]) applies to (all);
    GoodListOfPropRefPA1: list of Props::RecordType1 applies to (all);
    GoodListOfPropRefPA2: list of Props::RecordType2 applies to (all);

    BadListOfPropRefConstant1: constant list of Props::RecordType1 => (Props::R1_prop, Props::R2_prop, [f1 => 0;]);
    BadListOfPropRefConstant2: constant list of Props::RecordType2 => (Props::R1_prop, Props::R2_prop, [f1 => "hello";]);
    BadListOfPropRefDefault1: list of Props::RecordType1 => (Props::R1_prop, Props::R2_prop, [f1 => 0;]) applies to (all);
    BadListOfPropRefDefault2: list of Props::RecordType2 => (Props::R1_prop, Props::R2_prop, [f1 => "hello";]) applies to (all);
    BadListOfPropRefPA1: list of Props::RecordType1 applies to (all);
    BadListOfPropRefPA2: list of Props::RecordType2 applies to (all);
end Props;

P.aadl

package P
public
    with Props;

    abstract A
        properties
            Props::GoodPA1 => Props::C1;
            Props::GoodPA2 => Props::C2;

            Props::BadPA1 => Props::C2;
            Props::BadPA2 => Props::C1;

            Props::GoodListPA1 => (Props::C1, [f1 => 0;]);
            Props::GoodListPA2 => (Props::C2, [f1 => "hello";]);

            Props::BadListPA1 => (Props::C1, Props::C2, [f1 => 0;]);
            Props::BadListPA2 => (Props::C1, Props::C2, [f1 => "hello";]);

            Props::GoodFieldsPA1 => [q1 => Props::C1; q2 => Props::C2;];
            Props::GoodFieldsListPA => ([q1 => Props::C1; q2 => Props::C2;]);

            Props::BadFieldsPA1 => [q1 => Props::C2; q2 => Props::C1;];
            Props::BadFieldsListPA1 => ([q1 => Props::C2; q2 => Props::C1;]);

            Props::GoodPropRefPA1 => Props::R1_prop;
            Props::GoodPropRefPA2 => Props::R2_prop;

            Props::BadPropRefPA1 => Props::R2_prop;
            Props::BadPropRefPA2 => Props::R1_prop;

            Props::GoodListOfPropRefPA1 => (Props::R1_prop, [f1 => 0;]);
            Props::GoodListOfPropRefPA2 => (Props::R2_prop, [f1 => "hello";]);

            Props::BadListOfPropRefPA1 => (Props::R1_prop, Props::R2_prop, [f1 => 0;]);
            Props::BadListOfPropRefPA2 => (Props::R1_prop, Props::R2_prop, [f1 => "hello";]);
    end A;    
end P;
AaronGreenhouse commented 4 years ago

Really the problem is with name references as the property value.

AaronGreenhouse commented 4 years ago

Fixed this

Fixed error reporting to use better locations on the parse tree.

Added unit tests.