Vb6

Strings should not be set to empty string

vbNullString is a special constant that denotes a null string (0), while "" is a literal empty string. For most purposes, the two are equivalent, but vbNullString is faster to assign and process, and takes less memory. vbNullString is therefore preferred, however some non-VB APIs or components may not properly handle null strings, and their use should be tested.

Text = ""

Variable types should be specified

Declaring a variable without specifying its data type leaves the compiler to assign the type that seems the most appropriate - whether it’s what you need or not. Therefore you should always specify the data type.

Const MyVar = "459"
'...
Dim NumberOfEmployees

Loop invariants should not be calculated inside the loop

Loop invariants are expressions whose values do not change during the execution of a loop. Placing the calculation of an invariant inside a loop is confusing, and inefficient because the resulting value will always be the same, yet it must be re-calculated each time through the loop. Therefore invariants should be calculated and stored before loop execution begins.

For y = 0 to Height-1
For x = 0 to Width-1
  i = y*Width + x   ' y*Width is invariant
  '...
Next x
Next y

Static procedures should not be used

The local variables in a Static Sub or Function are preserved between calls (meaning that these variables have the same lifetime as the owning module of their procedure). Static procedures should be avoided because they are difficult to test, and can result in unexpected behavior.

Private Static Function Foo() As Single
Dim val As Single   ' val retains its value between invocations

val = val + 1
Foo = val

End Function

Expressions should not be too complex

The difficulty of understanding an expression increases for each of the And, Or and Xor operators it contains.

A single expression’s complexity should not become too high to keep the code readable.

If ( (condition1 And condition2) Or (condition3 And condition4) Or (condition5 And condition6) )
...
End If

Return type should be specified for functions

Error handlers should not declare their own error handlers

It is not possible to handle errors that arise inside error-handling code. Therefore, the declaration of a new error handler within an error-handling section will lead to unpredictable behavior, since the second error-handling routine is activated only after the first error handler has exited.

Sub InitializeMatrix(Var1 As Integer, Var2 As Integer, Var3 As Integer, Var4 As Integer)
On Error GoTo ErrorHandler
'. . .
Exit Sub
ErrorHandler:
On Error GoTo ErrorHandlerLastResort
'. . .
Resume Next
ErrorHandlerLastResort:
'. . .
Resume Next
End Sub

Forms should be unloaded and removed from memory

Because Visual Basic 6 does not automatically unload Form s after a program terminates, it is possible for them to remain in memory. Even after Unload ing, they may stay in memory if references remain. Therefore Form s should be explicitly Unload ed, and their references set to Nothing.

dim f1 as new form
f1.show
...
f1.hide

Variable data types should be declared explicitly

Variables declared without the explicit specification of a data type are Variants. Variants can be inefficient to use because at each interaction they are converted to the appropriate type for that interaction. Variants may be required for COM interactions, but even then their type should be specified explicitly.

Dim Count
Dim Bool

$ should not be used in string variable names

Select Case statements should have at least 3 Case clauses

`Select Case statements are useful when there are many different cases depending on the value of the same expression.

For just one or two cases however, the code will be more readable with If` statements.

Select Case number
Case 1 To 5
    MsgBox("Between 1 and 5, inclusive")
Case Else
    MsgBox("Not between 1 and 5, inclusive")
End Select

The & operator should be used to concatenate strings

Both the ”+” operator and the ”&” operator can be used to concatenate strings. However, there is a complicated set of rules around the actual behavior of the ”+” operator, and whether you will get a concatenation operation, addition, a compiler error, or a Type mismatch error. On the other hand, the ”&” operator can only perform concatenation, and is therefore preferred.

Dim y As String = "Con" + "caten" + "ation"

String-specific functions should be used

Variant functions are inefficient by nature, and should not be used with strings when string-specific versions, denoted by a `$ at the end of the name, are available.

This rule flags instances of these functions:

Left(), Mid(), Right(), Chr(), ChrW(), UCase(), LCase(), LTrim(), RTrim(), Trim(), Space(), String(), Format(), Hex(), Oct(), Str(), Error`

TrimString = Trim(TestString)

Strings should not be compared with empty string

It is far faster to compare the length of a string to 0 than it is to compare the string itself to empty string. Further, the LenB implementation is faster than the Len implementation, and is therefore preferred.

If Text = "" Then
If Text <> "" Then

GoTo statements should not be used

GoTo is an unstructured control flow statement. It makes code less readable and maintainable. Structured control flow statements such as If, For, While, or Exit should be used instead.

Sub gotoStatementDemo()
    Dim number As Integer = 1
    Dim sampleString As String
    ' Evaluate number and branch to appropriate label.
    If number = 1 Then GoTo Line1 Else GoTo Line2
Line1:
    sampleString = "Number equals 1"
    GoTo LastLine
Line2:
    ' The following statement never gets executed because number = 1.
    sampleString = "Number equals 2"
LastLine:
    ' Write "Number equals 1" in the Debug window.
    Debug.WriteLine(sampleString)
End Sub

Multiple variables should not be declared on the same line

Declaring multiple variable on one line is difficult to read and potentially misleading since the As typing only applies to the last variable on the line.

Public AAA, BBB, CCC As String ' AAA and BBB are variants

Asc[W] should not be called on string constants

Case clauses should not have too many lines

The Select statement should be used only to clearly define some new branches in the control flow. As soon as a Case clause contains too many statements this highly decreases the readability of the overall control flow statement. In such case, the content of Case clause should be extracted in a dedicated sub or function.

Select Case number
Case 1 To 5
    Debug.WriteLine("Between 1 and 5, inclusive")
Case 6, 7, 8
    Debug.WriteLine("Between 6 and 8, inclusive")
Case 9 To 10                 'Noncompliant, too many nested statements
    doSomething1()
    doSomething2()
    doSomething3()
    ...
    doSomethingN()
Case Else
    Debug.WriteLine("Not between 1 and 10, inclusive")
End Select

Chr[W]$() should not be used for certain characters

It is more efficient to skip the function invocation and use the predefined string constants rather than calling `Chr $() or ChrW$ () for the following numbers:

0 - vbNullChar
8 - vbBack
9 - vbTab
10 - vbLf
11 - vbVerticalTab
12 - vbFormFeed
13 - vbCr
13+10 - vbCrLf | vbNewLine
34 - ""`

Dim myChar As Char 
myChar = Chr$(0)

Type should be specified for parameters

Declaring a parameter without specifying its data type leaves the compiler to assign the type that seems the most appropriate - whether it’s what you need or not. Therefore you should always specify the data type.

Sub SubComputeArea(Length, TheWidth)
[. . .]
End Sub

Empty statements should be removed

Empty statements, i.e. `:, are usually introduced by mistake, for example because:

It was meant to be replaced by an actual statement, but this was forgotten.
There was a typo which resulted in the colon being doubled, i.e. ::`.

Dim i, j As Integer
i = 1 :: j=2   ' Noncompliant - double :

The bang (!) operator should not be used

Return of boolean expressions should not be wrapped into an if-then-else statement

Select statements should end with a Case Else clause

The requirement for a final Case Else clause is defensive programming. The clause should either take appropriate action or contain a suitable comment as to why no action is taken. Even when the Select covers all current values of an enum, a Case Else case should still be used because there is no guarantee that the enum won’t be extended.

Dim number As Integer = 8
Select Case number   'Non-Compliant, what to do when number is not between 1 and 10 ?
Case 1 To 5
    Debug.WriteLine("Between 1 and 5, inclusive")
    ' The following is the only Case clause that evaluates to True.
Case 6, 7, 8
    Debug.WriteLine("Between 6 and 8, inclusive")
Case 9 To 10
    Debug.WriteLine("Equal to 9 or 10")
End Select

Option Base should not be used

The use of Option Base to change the lower bound of an array’s index values results in confusing code.

Option Explicit
Option Base 1

'...
Dim MyArray(1 To 3) As Integer
For I = 1 To 3
    MsgBox MyArray(I)
Next I

Select Case statements should not be nested

Nested `Select Case structures are difficult to understand because you can easily confuse the cases of an inner Select Case as belonging to an outer statement. Therefore nested Select Case statements should be avoided.

Specifically, you should structure your code to avoid the need for nested Select Case statements, but if you cannot, then consider moving the inner Select Case` to another function.

Public Sub Foo(A As Integer, B As Integer)
Select Case A
    Case 0
        ' ...
    Case 1
        Select Case B   ' Noncompliant; nested Select Case
            Case 2
                ' ...
            Case 3
                ' ...
            Case 4
                ' ...
            Case Else
                ' ...
        End Select
    Case 2
        ' ...
    Case Else
        ' ...
End Select
End Sub

Statements should be on separate lines

Putting multiple statements on a single line lowers the code readability and makes debugging the code more complex.

Unresolved directive in <stdin> - include::{noncompliant}[]

Write one statement per line to improve readability.

Unresolved directive in <stdin> - include::{compliant}[]

sampleString  = "Hello World" : MsgBox(sampleString) ' Noncompliant

Magic numbers should not be used

Magic numbers make the code more complex to understand as it requires the reader to have knowledge about the global context to understand the number itself. Their usage may seem obvious when writing the code, but it may not be the case for another developer or later once the context faded away. -1, 0, and 1 are not considered magic numbers.

Function blnCheckSize(dblParameter As Double) As Boolean
If dblParameter > 1024 Then      ' Noncompliant, 1024 is a magic number
blnCheckSize = True
End If
End Function

Boolean literals should not be redundant

A boolean literal can be represented in two different ways: {true} or {false}. They can be combined with logical operators ({ops}) to produce logical expressions that represent truth values. However, comparing a boolean literal to a variable or expression that evaluates to a boolean value is unnecessary and can make the code harder to read and understand. The more complex a boolean expression is, the harder it will be for developers to understand its meaning and expected behavior, and it will favour the introduction of new bugs.

If BooleanVariable = True Then ...
If BooleanVariable <> True Then ...
If BooleanVariable OR False Then ...
DoSomething(Not False)

Mergeable if statements should be combined

Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as possible, by avoiding unnecessary nesting, is considered a good practice.

Merging if statements when possible will decrease the nesting of the code and improve its readability.

If a And b Then
If c And d Then           ' Noncompliant
doSomething()
End If
End If

Methods should not be empty

An empty {operationName} is generally considered bad practice and can lead to confusion, readability, and maintenance issues. Empty {operationName}s bring no functionality and are misleading to others as they might think the {operationName} implementation fulfills a specific and identified requirement.

There are several reasons for a {operationName} not to have a body:

It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
It is not yet, or never will be, supported. In this case an exception should be thrown.
The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

Sub ShouldNotBeEmpty()  ' Noncompliant - method is empty
End Sub

Sub NotImplementedYet()  ' Noncompliant - method is empty
End Sub

Sub WillNeverBeImplemented()  ' Noncompliant - method is empty
End Sub

Sub EmptyOnPurpose()  ' Noncompliant - method is empty
End Sub

Unused private variables should be removed

If a private field is declared but not used locally, its limited visibility makes it dead code.

This is either a sign that some logic is missing or that the code should be cleaned.

Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand and preventing bugs from being introduced.

Private Foo as Integer 'Noncompliant: Foo is unused and should be removed

Function Compute(A As Integer)
Compute = A * 42
End Function

Comments should not be located at the end of lines of code

This rule verifies that single-line comments are not located at the ends of lines of code. The main idea behind this rule is that in order to be really readable, trailing comments would have to be properly written and formatted (correct alignment, no interference with the visual structure of the code, not too long to be visible) but most often, automatic code formatters would not handle this correctly: the code would end up less readable. Comments are far better placed on the previous empty line of code, where they will always be visible and properly formatted.

a = b + c   ' This is a trailing comment that could be very very long  -- Noncompliant
d = a + 9 REM This is another trailing comment which could be very very long  -- Noncompliant
e = a * b ' FIXME  -- allowed

Sub and function names should comply with a naming convention

Using hardcoded IP addresses is security-sensitive

Hardcoding IP addresses is security-sensitive. It has led in the past to the following vulnerabilities:

Today’s services have an ever-changing architecture due to their scaling and redundancy needs. It is a mistake to think that a service will always have the same IP address. When it does change, the hardcoded IP will have to be modified too. This will have an impact on the product development, delivery, and deployment:

The developers will have to do a rapid fix every time this happens, instead of having an operation team change a configuration file.
It misleads to use the same address in every environment (dev, sys, qa, prod).

Last but not least it has an effect on application security. Attackers might be able to decompile the code and thereby discover a potentially sensitive address. They can perform a Denial of Service attack on the service, try to get access to the system, or try to spoof the IP address to bypass security checks. Such attacks can always be possible, but in the case of a hardcoded IP address solving the issue will take more time, which will increase an attack’s impact.

Private Sub cmdConnect_Click()
sockMain.RemoteHost = txtHost.Text
sockMain.RemotePort = txtPort.Text
sockMain.Connect
End Sub

Track uses of TODO tags

Developers often use TODO tags to mark areas in the code where additional work or improvements are needed but are not implemented immediately. However, these TODO tags sometimes get overlooked or forgotten, leading to incomplete or unfinished code. This rule aims to identify and address unattended TODO tags to ensure a clean and maintainable codebase. This description explores why this is a problem and how it can be fixed to improve the overall code quality.

Rem TODO Add documentation
Sub DoSomething()
' TODO
End Sub

Source code should be indented consistently

Consistent indentation is a simple and effective way to improve the code’s readability. It reduces the differences that are committed to source control systems, making code reviews easier.

This rule raises an issue when the indentation does not match the configured value. Only the first line of a badly indented section is reported.

Public Sub OutputObject(oOutput As Object)
Set oObject = oOutput

If TypeOf oObject Is Form Then
oObject.Cls
ElseIf TypeOf oObject Is PictureBox Then
    oObject.Cls
ElseIf TypeOf oObject Is ListBox Then
    oObject.Clear
End If
End Sub

Start Here

Setup

Pull Request Review

Scan center

Integrations

IDE

Rule Reference

Resources