New rules for ObjectScriptQuality

good point:
there are only  17 rules referring to BUGS:

In total the quality check shows 106 rules

So there is more and I personally deeply disagree with some of them

I had a look to the various rules, including non BUGS, and in some case I don't fully agree to the rule, but I understand sometimes it's a matter of opinion.

There are however cases where the rule are simply wrong, other cases where the supplied sample is wrong.

I haven't checked all the rules (yet? 😊), the errors I found so far are:
 

Rule Property name declared with quotes
It's funny that the "Compliant Solution" is simply an invalid/wrong property definition! 😱

Property Hello_World;

While I fully agree to avoid quoted property declarations and the use of "non standard" property names (like "Hello_World") I can imagine some scenario where this is definitely required and cannot be avoided.
 

Rule Incompatible argument type in a method
According to the description, running the provided sample code, in two cases, call to DoWork2() and DoWork3() methods, raise a "PARAMETER error" message on runtime.
In fact, the supplied code never raise any error.
 

Has anyone ever checked/reviewed the rules and found dubious rules or don't fully agree with some rule?

Some more notes about ObjectScriptQuality rules.

Rule: SQL Delimited Identifiers with double quotes

Both the "Non-compliant Code Example" and "Compliant Solution" for the Class Query sample contain an invalid SQL statement.
For example the Compliant Solution for the Class Query sample is:

Query Example() As %SQLQuery(CONTAINID = 1, ROWSPEC = "ID,Corporation:%Integer,IDNumber:%Integer,Name:%String, DisplayName:%String")
{
  SELECT ID, Corporation->CorporationID, IDNumber, Name, (IDNumber || ' - ' || Name) As
    FROM General.Contacts
  ORDER BY Name
}

At the end of the first line of the select statement the column alias is missing after "As ...".

In othe samples, I think it's misleading the naming of "SomeCondition" where in fact it represents a value:
 

&SQL(SELECT Val1, Val2
                INTO :val1, :val2
                FROM Table
                WHERE Val1='SomeCondition')

Here 'SomeCondition' is not a condition, it's a value, I'd rather use/name it 'SomeValue'.
I refrain from commenting that in the sample SQL above, the query returns a constant as first column/value (Val1).....

Rule: Use of &sql

The rule states:

The problem with using &sql(...) is that the execution plan will be calculated the first time the query is executed, and never be re-evaluated again.2

This was true, however since 2020.1 IRIS uses "Universal Query Cache" for both embedded SQL and Dynamic Queries.
If &sql() should be avoided (and this can be discussed...), a better, strong, valid, true reason should be provided.

Rule: Use of OPEN is discouraged

The OPEN command has a very complicated syntax which is difficult to get right.

It is preferable to use classes from the %IO package instead which are much easier to use and provide the same functionality.

The proposed solution breaks rule Undocumented class/method

The %IO package is never mentioned in the documentation and is (by default) hidden in class reference.
Depending on the device, different documented classes can be used instead of %IO package.

Maybe we can discuss/ask InterSystems what are the plans for %IO package.
Is it here to stay? Is it going to be deprecated? Is it going to be documented?

More importantly, can we safely use it? Sometime (often?) undocumented means that should not be used by "user code" and/or it can be changed (by ISC) without notice in any future version.