CachéQuality for VSCode now available

Hope most of you already familiar with project CachéQuality from @Daniel Tamajon. For those who don’t know about it, it is a static syntax analyzer for your code written for InterSystems products. It may help you to find and solve many different types of issues in your code, and even possible bugs before clients will find it in production. So, with help of CachéQuality you will be able to deliver a better product. You can find the complete list of rules used to check ObjectScript code here.

It was already available in Studio. And now it is also available in VSCode.

Installation

In Extension section of VSCode Search for cachequality and install SonarLint for CachéQuality. Make sure you installed VSCode for ObjectScript first to get a better experience. Note, you can find both extensions by searching for objectscript.

So, right after installing this extension you will be able to use without any configuration, and after opening any class, it will underline all found issues.

And you can hover underlined lines, to see some details.

You can also open details for the problem right in the VSCode.

VSCode has problems view,  which can be activated by Cmd+Shift+M hotkey, and this view used to show a list of problems found in your opened source files. And you can find there a list of problems found by CacheQuality and go to the exact place.

Fixed lines will be rechecked after save

There are some limitations though. You aren't able to control the list of rules and some other parameters related to those rules. Extension delivered as preconfigured with default values. If you need more control you can use connected mode.

Connected mode

This extension also has such mode named as connected. In this mode, you can connect to your SonarQube server with installed CacheQuality plugin

With SonarQube you can customize the list of rules. Disable or activate, and change the rule's parameters. For example, rule `To method has too many lines` by default triggered after 50 lines, and you will be able to change this number, and so on.

When you have SonarQube Server, you have to first analyze the whole of your project. There are different ways how to do do it, you can find details in the SonarQube documentation. And you need a token to get the access to your project from VSCode.

Change VSCode  settings

    "sonarlint.connectedMode.project": {
        "projectKey": "Samples",
        "serverId": "local"
    },
    "sonarlint.connectedMode.servers": [
        {
            "serverId": "local",
            "serverUrl": "http://localhost:9000",
            "token": "65b19eb2ef04cd81a033c89820acf65d1f349c4f"
        }
    ]

Keep in mind that projectKey should be equal with defined on SonarQube, serverId from the list of the list defined in servers section.

After saving settings and after any changes on SonarQube side, you should update  the binding, to make sure your VSCode side is up to date.

If the settings were updated correctly you’ll see the following notification:

If you would like to customize the list of used rules for your project, first you have to copy the built-in quality profile Caché Quality.

And activate the new profile to the project.

You may resolve some found issues in SonarQube, and VSCode extensions will see it and will not show it again.

For example, we have such problems. 

Let's resolve the issue with the never invoked method as False Positive, and we a not going to fix the issue with the variable.

After reopening file, it will be reanalyzed with new information. And method %OnNew does not have unresolved issues anymore.

Issues and feedback

CacheQuality project itself is not open source, but the extension for VSCode is open. And you can fill any issues and feedback there.

Comments

As a customer of CacheQuality/Lite Solutions I have been working with @Daniel Tamajon and staff.  They have been very responsive to all inquiries and responded incredibly fast any time I brought a bug or request to their attention.  The plugin for VSCode works very well and has been helping me to identify code smells as I am developing, prior to checking in for full SonarQube scanning.

I highly recommend everyone give it a shot, it works seamlessly with the VSCode extension.

Just my 2 cents.

Any way to suppress warnings without sonarQube?

//@SupressWarnings("cachequality:OS0001")

//@SupressWarnings("all")

or 

//NOSONAR

is not working

We do not have any way to suppress warnings, yet. As I know only SonarQube for Java supports it.

Would be nice to have it. 

$$$TOE(sc, method()) gives a warning "Consider inlining/replacing this macro to make the code more readable".

It is more readable but I want to use the macro: 

set sc = method()

if ('sc) {

throw ##class(%Exception.StatusException).CreateFromStatus(sc)

}

It's just a rule to eliminate macros in my opinion :/

Other example:

Method has too many lines (93 > 50). Am I not allowed to write methods bigger 50 lines?

Method declares too many variables? Also a rule I can't understand. Sure you need to minimize but it's not always possible. 

I really like sonarlint but I can't use it at 100% because I can't disable specific rules in specific circumstances, 

For now, sonarlint for VS Code does not allow to disable rules. Currently, this feature is only available on Atelier.

All the referred parameters are configurable from SonarQube, where you can define multiple Quality Profiles (it is, different sets of rules). So when you get your sonarlint connected to Sonarqube you will use the Profile definition for the particular project.

Otherwise, you are using the default settings which can be modified on a different way if many of you require it to us, but it will never fit to all projects as each one has its own particularities.

You can rise it on https://tracker.cachequality.com

Very nice. I like the Objectscript plugin in VSCode and using it already. Great to have this too !

Why the construction:

if condition do ..Method() 

is considered as a bad practice?

IMHO it's more readable than :

If condition {

 do ..Method()

}

I have plenty of this, e.g. here:

if globalPath[".gz"  do ..importXMLGlobal(globalPath)
  if globalPath[".xml" do $System.OBJ.Load(globalPath)

Can I turn off the rule?

Hmmm,
it seems to prefer "waste screen space" style 
over "have all on 1 screen with no scrolling"   [my favorite ]

Don't like post conditions.

And anyway - for this you'll get from CacheQuality:

"Consider using an If statement instead of a postconditional (cachequality:OS0039)"

with description:

"This feature of the language may lead to shorter code overall; however, users unfamiliar with the language may have trouble understanding what this syntax means.

For understandability reasons, it is preferred to use an if statement instead."

;)

I remember times when new programmers learned from their masters to use no matter what
programming language in her full beauty and elegance with all its features.
no matter if this was Assembler/360 or Macro32 or C, C++, C# or Bliss or PL/1 or Fortran, Algol, .... [list almost unlimitted]

All of them had their tricks and open and hidden features that inspired the creativity of developers
and allowed them to create artwork instead of stereotypic formulated phrases. By this approach,
developers get degraded from architects to monotone brick assemblers.

"users unfamiliar with the language" 
I'd recommend they should learn it to understand it before touching it.
If I want to read the Правда  I have to learn Cyrillic characters and the Russian language to understand it.
Nobody would accept any request for Latin letters.

I agree with "they should learn it to understand it before touching it", but we are not talking to go from Latin to Cyrillic... many people is working in Java, C#, Javascript, ... which, let's say, all of them are not just Latin but also Romanic... so after you learn a second language the third one is really easy because you know how grammatical structure works and you only need learn about semantics.

So, in our consideration and accordingly also with community users and customers that helped us to consider each rule, if the language allows a way to be more "Romanic", it's a bad practice not to use that way.

This consideration is not absolute in any way, just a recomendation.

Just to say that not all the rules are valid for all the projects/context/environments, as the development guide can differs from one company to another (even from one project to another in the same company). So the most important is to find the useful rules for each context and configure the proper profile to allow certificate the code with your own quality requirements.

For sure I do not want to return to the coding style like 

 S f="inputfile.txt" O f U f F  R l D

 . Q:l="***END***"

 . D ##class(process).Input(l)

But working with Python seems to be broadly accepted all over the globe without questioning the syntax of it. In other words: I agree with Robert. There is no perfect programming language and it is always painful to read a code written in an "unknown" language. On the other hand it does not mean that a programmer should not care about code maintainability. So what is the point? Everyone must find the balance between coding efficiency and readability. And tools are just tools helping us find the balance. 

returning to dotted subroutines would be really bad. Though they are still around in %SYS.  laugh

on the other hand, the flexibility of the language allows a broad range of personal styles.
knowing them enabled me in the past (amongst my customers ) to identify the author of a routine
just by his style with a hit rate of >75%.  devil

How can I code without:

quit:$$$ISERR(sc) sc

it's basically 5% of the code I write.

Accordingly to the different rules applied, it should be rewritten like this:

if $$$ISERR(sc) { return sc }

You can check it on the rule help provided in the same environment or here.

But I think in this case should be:

return:$$$ISERR(sc) sc

I believe it's always the return from the method. Right, Ed?

From docs. RETURN and QUIT differ when issued from within a FOR, DO WHILE, or WHILE flow-of-control structure, or a TRY or CATCH block.

  • You can use RETURN to terminate execution of a routine at any point, including from within a FOR, DO WHILE, or WHILE loop or nested loop structure. RETURN always exits the current routine, returning to the calling routine or terminating the program if there is no calling routine. RETURN always behaves the same, regardless of whether it is issued from within a code block. This includes a TRY block or a CATCH block.
  • In contrast, QUIT exits only the current structure when issued from within a FOR loop, a DO WHILE loop, a WHILE loop, or a TRY or CATCH block. QUIT exits the structure block and continues execution of the current routine with the next command outside of that structure block. QUIT exits the current routine when issued outside of a block structure or from within an IF, ELSEIF, or ELSE code block.

Hi, Ed!

it’s good that you cited docs here, all is fair.

but you didn’t answer my question on the cases you use this particular expression. 

And considering that it checks the error status it mostly means that you quit from the method with it, right?

so this should be return and not quit. Right?

No, because as those

 quit sc

ever worked, they were used according to the rule cited above:

outside of a block structure or from within an IF, ELSEIF, or ELSE code block.

This is very personal, but I would never use RETURN, because it introduces backward incompatibility with older Caché versions without any real reason for it but using yet another "me too" "modern" feature. Besides, it provokes the developer to bypass modular approach rule that insists on having one and only one enter as well as exit from each functional module. Isn't it about real (rather than "me too") readability?

Again, that's only IMO, while I doubt if it's possible to issue any community accepted rule set for COS programming style.

Alexey, I do not argue the difference, between "return" and "quit". There is a difference.

In this case it's a guess that if we check the error and return the value with "quit" it's 100% case that it is a quit from the method, rather then any other use cases of "quit".

So it is "return". @Eduard Lebedyuk ?

Answering your point - IMO "return" is a more readable command to get a direct answer on "What this method returns" question.

Evgeny, agree with you, there is the difference, while I'm not sure what percent of community members will give the right answer to the question below without looking into docs and running the code. AFAIK, voting is impossible here.

What result will be returned by the method:

ClassMethod t1()
 {
  do t2(.a,.b)
  do
  . set a=4,b=5
  . return $increment(a)*$increment(b)
  return $increment(a,-1)*$increment(b,-1)
t2(&pA,&pB) ;
  set pA=2,pB=3
  return pA*pB
 }

Nice example, Alexey)

But I wouldn't use either dot-syntax or "do label" in new ObjectScript code.

This could be changed with ObjectScript best practices:

ClassMethod t1() as %Integer 

{ 

 do ..t2(.a,.b) 

  n a,b

  set a=4,b=5
  return $increment(a)*$increment(b)

}

ClassMethod t2(ByRef pA, ByRef pB) as %Integer

{

 set pA=2,pB=3
  return pA*pB

}

Which, IMO, gives you more clear understanding what the code does.

Evgeny,

Thank you for teaching me COS :)

Just a small note to finish it:

  • The rewritten sample is not functional equivalent of the original one and will return the different result (if correct the syntax error).
  • My sample was not about "old" syntax vs. "new" one; I just wanted to emphasize that simple substitute "quit" with "return" command either may or may not improve the readability of already existing class/routine, it all depends...

Hi Alexey!

Thank you for teaching me COS :)

Didn't even try, you know ;) 

But for those who see your code sample, I, as a Community Manager, must say that dot syntax and "do label" is never a good practice in ObjectScript today. 

And I can't imagine that InterSystems product manager who introduced return in ObjectScript could see it called from dot-level ;)

I think that we have {} in ObjectScript to excuse the dot-syntax usage and do ..Method() for do Label cases. 

The rewritten sample is not functional equivalent of the original one and will return the different result (if correct the syntax error).

This illustrates how dot-syntax could easily obfuscate the logic if you want ;)

My sample was not about "old" syntax vs. "new" one; I just wanted to emphasize that simple substitute "quit" with "return" command either may or may not improve the readability of already existing class/routine, it all depends...

Agree. 

But IMHO for a new ObjectScript code in InterSystems IRIS return value is always more readable than quit value.

And I think in majority cases, where you don't use "dot-syntax" this replacement could be applicable too.  @Eduard Lebedyuk  - is it eligible for your 

Quit: $$$IsError(sc) sc

case? 

And to All: this is not an instruction to action!

This is the discussion and activity to introduce a Developer Community Coding Guidelines Best Practices for InterSystems IRIS ObjectScript. You may use this or may not but it's good to have it.

P.S. I think we need to stop shrinking the levels of the thread in DC engine :)

I never use return unless I really do want to exit from the method while inside some inner loop. So mainly I use quit.

First of all, you can only enable or disable rules from SonarQube, so you need your sonarlint connected to a SonarQube server to user your own set of rules. SonarLint is not ready to do that on VS Code while in Atelier can be done.

In that case, readability refers to usage of "{" and "}", as you enclose the execution of the "if" evaluation and helps to understand which instructions will be executed on true evaluation.

I will expose our considerations next, and we are opened to change it if community holds on a different suggestion, or better we can create an alternative rule to accomplish a different requirement.

You can either have many statements on one line, all of the will be executed on true evaluation, really hard to read if you have many statements:

if condition do ..firstThing() do ..secondThing()

Or you could try to write:

if condition
    do ..something()

In that case, the true evaluation will execute nothing, and the next sentence will be always executed.

Nowadays we write in many languages and we need to have a deep knowledge to do good stuff, but also we use many times the same structures, as they are more easy on remember, so we can have "silly" errors because some languages have a more strict structure than others.

And if you want to allow developers using other languages to write proper ObjectScript, as more similar is semantics to other languages, more easy will be to them to adopt to ObjectScript.

Hola Daniel!

Thank you for your explanation! And thank you for a really great tool!

if condition
    do ..something()

I agree, and this is not a good practice - two lines with if.

But nobody does that in ObjectScript. As developers consider spaces in Python so developer follows the nuances of the ObjectScript. And oneliner

if condition do ..something()  

is considered as a readable and convenient way to code ObjectScript.

As well as postconditions on 

quit:$$$ISERR(sc) sc

Daniel!

I suggest to let Developer Community vote for the rules and this will let Community have a general version of adopted ObjectSript coding guidelines along with a general tool - CacheQuality which can so nicely help to control these guidelines.  What do you think?

Hi Evgeny!

I think it's a nice idea to have a default profile based on Community voted rules, which will allow proper usage of Studio/Atelier/VSCode plugins without a SonarQube server.

Also, we encourage anyone to propose new helpful rules or (as we are doing here) to discuss about existing ones to get a revised version of them based on Community consensus.

that's wrong !  "will execute nothing, "   

if condition

Sets the system variable $TEST.   
And this was and still is a widely used way of signaling between routines and functions.
And millions lines of existing code rely on its proper use.

I come back to an earlier comment: Really learning the language is definitely an advantage.

You are right Robert, I should say "nothing apparently" as there is always an implicit execution to set $TEST on legacy IF (but not in new IF version).

Agree with "learning the language is and advantage", and tools should help to make an easier learning curve.