New rules for ObjectScriptQuality
Hi Developers!
There is a free ObjectScriptQuality tool for ObjectScript developers who upload solutions on GitHub. This tool helps developers validate ObjectScript code using a variety of rules, based on common code errors.
There are currently 16 rules in this tool, but there are definitely many more rules to consider when testing ObjectScript code.
Could you suggest any other rules to add to this tool?
Note: You can submit it as an idea on the InterSystems Ideas for the 2nd Idea-A-Thon contest. Source control tools help developers reduce the time to test, develop and deploy applications, so improving these tools is in line with the topic of the contest.
Comments
good point:
there are only 17 rules referring to BUGS:
In total the quality check shows 106 rules.png)
So there is more and I personally deeply disagree with some of them
@Robert Cemper , this is why we created the post - let's discuss. As the whole idea of the project to help in building more readable and safe ObjectScript code. Your input is very important.
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?
@Enrico Parisi , thanks for reviewing the rules! I request @Dmitry Maslennikov and @Daniel Tamajon to comment.
It'd be great also if you could suggest the new objectscript scanning rules that will help developers.
I met some issues in the past, where I miss any idea of how to check
- verification of %variables or variables in global scope
- variables set as 3rd parameter in $QUERY() or $ORDER() - should be easier to detect.
- not sure if variables passed ByRef or Output are fixed meanwhile
About quoted properties, from my experience, the only scenario where those properties can be used is with JSON, but it's not necessary there anyway and can be declared differently.
The point was that the "Compliant Solution" is actually invalid code/definition.
I fully agree that quoted property declarations should be avoided.
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
FROMTable
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.
Hi @Enrico Parisi !
Where do you see these rules? I agree, they are quite strange, but some customers of ObjectScript quality use them.
Anyway, the rule set we use for the community is here - there are only 12 rules currently.
Hi
I found the rules here.
I understand that at the moment only some of the rules are applied to Open Exchange projects, but as you said, some customer may use the other rules to measure code quality or maybe simply to learn how to write good quality code.
Having published "wrong rules", whatever is the use, I don't think is a good thing. Fix the rule or remove it, before someone think they are good practice or useful info.
Yes, @Enrico Parisi ! I also have questions to the "full objectscript rules" list. And I hope @Daniel Tamajon and @Dmitry Maslennikov can follow up.
Anyway, if you and other developers have any ideas what else could be added to a short "community list" it'd be rather helpful for the whole community.
We were 'forced' to use CQ by one of our customers and I was rather disappointed about it.
It looks like the designers of those rules didn't have a lot of COS (or even MUMPS) experience.
Maybe we should, as a community, find some kind of consensus what is good (?) (C)OS code.
ISC is not a good example, their code (completely open for developers to look at, which you find nowhere in the industry): a lot of different styles (don't mention the 'p" and 't' prefixes ... ;-))
I don't have the time to set this up, but am happy to join the discussions.
Hi everyone,
I'd like to propose a short remote meeting to briefly explain the logic behind the current ObjectScript quality system. The goal is to ensure we all share the same foundation to continue this open discussion on what we consider "good" or "bad" code.
From there, we can collectively evaluate, refine, or propose new rules based on real needs and shared insights.
It's important to note that the current set of rules was developed thoughtfully, with input from experienced developers and in alignment with customer expectations.
Please let me know your availability (via private message) so we can schedule the meeting accordingly.
Looking forward to your feedback and participation.