Get Started
- CodeAnt AI
- Setup
- Control Center
- Pull Request Review
- IDE
- Compliance
- Anti-Patterns
- Code Governance
- Infrastructure Security Database
- Application Security Database
Vbnet
A cast is an explicit conversion, which is a way to tell the compiler the intent to convert from one type to another.
In Visual Basic, there are two explicit conversion operators:
Public Sub Method(Value As Object)
Dim i As Integer
i = DirectCast(Value, Integer) ' Direct casting from object holding an integer type to Integer
i = CType(Value, Integer) ' Conversion from the underlying type to Integer
End Sub
Functions can return values using two different syntaxes. The modern, and correct, way to do it is to use a `Return statement. The VB6 way, i.e. old way, is to assign a return value to the function’s name .
The VB6 syntax is obsolete as it was introduced to simplify migration from VB6 projects. The compiler will create a local variable which is implicitly returned when execution exits the function’s scope.
Return` statement should be used instead as they are easier to read and understand.
Public Function FunctionName() As Integer
FunctionName = 42 ' Noncompliant
End Function
Public Function FunctionNameFromVariable() As Integer
Dim Value As Integer = 42
FunctionNameFromVariable = Value ' Noncompliant
End Function
There’s no point in creating an array solely for the purpose of passing it to a ParamArray parameter. Simply pass the elements directly. They will be consolidated into an array automatically.
Class SurroundingClass
Public Sub Base()
Method(New String() { "s1", "s2" }) ' Noncompliant: unnecessary
Method(New String(12) {}) ' Compliant
End Sub
Public Sub Method(ParamArray args As String())
' Do something
End Sub
End Class
Short-circuit evaluation is an evaluation strategy for Boolean operators, that doesn’t evaluate the second argument of the operator if it is not needed to determine the result of the operation.
VB.NET provides logical operators that implement short-circuiting evaluations AndAlso and OrElse, as well as the non-short-circuiting versions And and Or. Unlike short-circuiting operators, the non-short-circuiting operators evaluate both operands and afterward perform the logical operation.
For example False AndAlso FunctionCall always results in False even when the FunctionCall invocation would raise an exception. In contrast, False And FunctionCall also evaluates FunctionCall, and results in an exception if FunctionCall raises an exception.
Similarly, True OrElse FunctionCall always results in True, no matter what the return value of FunctionCall would be.
The use of non-short-circuit logic in a boolean context is likely a mistake, one that could cause serious program errors as conditions are evaluated under the wrong circumstances.
If GetTrue() Or GetFalse() Then ' Noncompliant: both sides evaluated
End If
Complex boolean expressions are hard to read and so to maintain.
If ((condition1 AndAlso condition2) OrElse (condition3 AndAlso condition4)) AndAlso condition5) Then 'Noncompliant
...
End If
In most cases, indexed properties should be named Item for consistency. Exceptions are when there exists a name which is obviously better, for example System.String.Chars(System.Int32).
Module Module1
Dim array = {"apple", "banana", "orange", "strawberry"}
ReadOnly Property Foo(ByVal index As Integer) ' Noncompliant
Get
Return array(index)
End Get
End Property
End Module
A loop statement with at most one iteration is equivalent to an If statement; the following block is executed only once.
If the initial intention was to conditionally execute the block only once, an If statement should be used instead. If that was not the initial intention, the block of the loop should be fixed so the block is executed multiple times.
A loop statement with at most one iteration can happen when a statement unconditionally transfers control, such as a jump statement or a throw statement, is misplaced inside the loop block.
This rule raises when the following statements are misplaced:
Public Function Method(items As IEnumerable(Of Object)) As Object
For i As Integer = 0 To 9
Console.WriteLine(i)
Exit For ' Noncompliant: loop only executes once
Next
For Each item As Object In items
Return item ' Noncompliant: loop only executes once
Next
Return Nothing
End Function
Using the With statement for a series of calls to the same object makes the code more readable.
Module Module1
Dim product = New With {.Name = "paperclips", .RetailPrice = 1.2, .WholesalePrice = 0.6, .A = 0, .B = 0, .C = 0}
Sub Main()
product.Name = "" ' Noncompliant
product.RetailPrice = 0
product.WholesalePrice = 0
product.A = 0
product.B = 0
product.C = 0
End Sub
End Module
When an assembly uses Windows Forms (classes and interfaces from the `System.Windows.Forms namespace) its entry point should be marked with the STAThreadAttribute to indicate that the threading model should be “Single-Threaded Apartment” (STA) which is the only one supported by Windows Forms.
This rule raises an issue when the entry point (Shared Sub Main` method) of an assembly using Windows Forms is not marked as STA.
Imports System.Windows.Forms
Public Class Foo
Shared Sub Main()
Dim winForm As Form = New Form
Application.Run(winForm)
End Sub
End Class
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all non-private Const field names comply with the provided regular expression.
The default configuration is the one recommended by Microsoft:
Pascal casing, starting with an upper case character, e.g. BackColor
Short abbreviations of 2 letters can be capitalized, e.g. GetID
Longer abbreviations need to be lower cased, e.g. GetHtml
Module Module1
Public Const foo = 0 ' Noncompliant
End Module
The DebuggerDisplayAttribute is used to determine how an object is displayed in the debugger window.
The DebuggerDisplayAttribute constructor takes a single mandatory argument: the string to be displayed in the value column for instances of the type. Any text within curly braces is evaluated as the name of a field or property, or any complex expression containing method calls and operators.
Naming a non-existent member between curly braces will result in a BC30451 error in the debug window when debugging objects. Although there is no impact on the production code, providing a wrong value can lead to difficulties when debugging the application.
This rule raises an issue when text specified between curly braces refers to members that don’t exist in the current context.
<DebuggerDisplay("Name: {Name}")> ' Noncompliant - Name doesn't exist in this context
Public Class Person
Public Property FullName As String
End Class
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all even names match a provided regular expression.
The default configuration is the one recommended by Microsoft:
Pascal casing, starting with an upper case character, e.g. BackColor
Short abbreviations of 2 letters can be capitalized, e.g. GetID
Longer abbreviations need to be lower cased, e.g. GetHtml
Class Foo
Event fooEvent() ' Noncompliant
End Class
Because parameter names could be changed during refactoring, they should not be spelled out literally in strings. Instead, use `NameOf(), and the string that’s output will always be correct.
This rule raises an issue when any string in the Throw` statement is an exact match for the name of one of the method parameters.
Public Sub DoSomething(param As Integer, secondParam As String)
If (param < 0)
Throw New Exception("param") ' Noncompliant
End If
If secondParam is Nothing
Throw New Exception("secondParam should be valid") ' Noncompliant
End If
End Sub
Using Thread.Sleep in a test might introduce unpredictable and inconsistent results depending on the environment. Furthermore, it will block the thread, which means the system resources are not being fully used.
<TestMethod>
Public Sub SomeTest()
Threading.Thread.Sleep(500) ' Noncompliant
' assertions...
End Sub
Creating a new Exception without actually throwing it is useless and is probably due to a mistake.
If x < 0 Then
Dim ex = New ArgumentException("x must be nonnegative")
End If
If you’re using a Structure, it is likely because you’re interested in performance. But by failing to implement IEquatable<T> you’re loosing performance when comparisons are made because without IEquatable<T>, boxing and reflection are used to make comparisons.
Structure MyStruct ' Noncompliant
Public Property Value As Integer
End Structure
Other than `Exit Select, using an Exit statement is never a good idea.
Exit Do, Exit For, Exit Try, and Exit While will all result in unstructured control flow, i.e. spaghetti code.
Exit Function, Exit Property, and Exit Sub are all poor, less-readable substitutes for a simple return, and if used with code that should return a value (Exit Function and in some cases Exit Property) they could result in a NullReferenceException.
This rule raises an issue for all uses of Exit except Exit Select and Exit Do` statements in loops without condition.
Public Class Sample
Dim condition As Boolean
Public Sub MySub()
If condition Then
Exit Sub ' Noncompliant
End If
For index = 1 To 10
If index = 5 Then
Exit For ' Noncompliant
End If
' ...
Next
End Sub
Function MyFunction() As Object
' ...
MyFunction = 42
Exit Function ' Noncompliant
End Function
End Class
Unsigned integers have different arithmetic operators than signed ones - operators that few developers understand. Therefore, signed types should be preferred where possible.
Module Module1
Sub Main()
Dim foo1 As UShort ' Noncompliant
Dim foo2 As UInteger ' Noncompliant
Dim foo3 As ULong ' Noncompliant
End Sub
End Module
Certain bitwise operations are not needed and should not be performed because their results are predictable.
Specifically, using And -1 with any value always results in the original value.
That is because the binary representation of -1 on a numeric data type supporting negative numbers, such as Integer or Long, is based on two’s complement and made of all 1s: &B111…111.
Performing And between a value and &B111…111 means applying the And operator to each bit of the value and the bit 1, resulting in a value equal to the provided one, bit by bit.
anyValue And -1 ' Noncompliant
anyValue ' Compliant
Calling ToString() on an object should always return a string. Thus, overriding the ToString method should never return Nothing because it breaks the method’s implicit contract, and as a result the consumer’s expectations.
Public Overrides Function ToString() As String
Return Nothing ' Noncompliant
End Function
Shared naming conventions allow teams to collaborate efficiently. This rule checks that all non-private Shared ReadOnly fields names match a provided regular expression.
The default configuration is:
Pascal casing, starting with an upper case character, e.g. BackColor
Short abbreviations of 2 letters can be capitalized, e.g. GetID
Longer abbreviations need to be lower cased, e.g. GetHtml
Class Foo
Public Shared ReadOnly foo As Integer ' Noncompliant
End Class
Numbers can be shifted with the <[/code> and <code]> operators, but the right operand of the operation needs to be an int or a type that has an implicit conversion to int. However, when the left operand is an object, the compiler’s type checking is turned off, therfore you can pass anything to the right of a shift operator and have it compile. If the argument can’t be implicitly converted to int at runtime, a RuntimeBinderException will be raised.
Dim o As Object = 5
Dim x As Integer = 5
x = o >> 5 ' Noncompliant
x = x << o ' Noncompliant
Using the same value on either side of a binary operator is almost always a mistake. In the case of logical operators, it is either a copy/paste error and therefore a bug, or it is simply wasted code, and should be simplified. In the case of most binary mathematical operators, having the same value on both sides of an operator yields predictable results, and should be simplified.
This rule ignores *, +, &, <<, and >>.
If (a = a) Then
doZ()
End If
If a = b OrElse a = b Then
doW()
End If
Dim j = 5 / 5
j = 5 \ 5
j = 5 Mod 5
Dim k = 5 - 5
Dim i = 42
i /= i
i -= i
Array literals are more compact than array creation expressions.
Module Module1
Sub Main()
Dim foo = New String() {"a", "b", "c"} ' Noncompliant
End Sub
End Module
Visual Basic .NET offers a non-short-circuit conditional function, `IIf(), which returns either its second or third parameter based on the expression in the first parameter. Using it is slower than using If() because each parameter is unconditionally evaluated. Further, its use can lead to runtime exceptions because IIf always evaluates all three of its arguments.
The newer version, If()`, should be used instead because it short-circuits the evaluation of its parameters.
Public Class Foo
Public Sub Bar()
Dim var As Object = IIf(Date.Now.Year = 1999, "Lets party!", "Lets party like it is 1999!") ' Noncompliant
End Sub
End Class
A chain of If/ElseIf statements is evaluated from top to bottom. At most, only one branch will be executed: the first statement with a condition that evaluates to True. Therefore, duplicating a condition leads to unreachable code inside the duplicated condition block. Usually, this is due to a copy/paste error.
The result of such duplication can lead to unreachable code or even to unexpected behavior.
If param = 1 Then
OpenWindow()
ElseIf param = 2 Then
CloseWindow()
ElseIf param = 1 Then ' Noncompliant: condition has already been checked
MoveWindowToTheBackground() ' unreachable code
End If
Using operator pairs (=+ or =-) that look like reversed single operators (+= or -=) is confusing. They compile and run but do not produce the same result as their mirrored counterpart.
Dim target As Integer = -5
Dim num As Integer = 3
target =- num ' Noncompliant: target = -3. Is that the intended behavior?
target =+ num ' Noncompliant: target = 3
Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate.
This rule allows to check that all parameter names match a provided regular expression.
The default configuration is the one recommended by Microsoft:
Camel casing, starting with a lower case character, e.g. backColor
Short abbreviations of 2 letters can be capitalized only when not at the beginning, e.g. id, productID
Longer abbreviations need to be lower cased, e.g. html
Module Module1
Sub GetSomething(ByVal ID As Integer) ' Noncompliant
End Sub
End Module
The size of a collection and the length of an array are always greater than or equal to zero. Testing it doesn’t make sense, since the result is always true.
If Collection.Count >= 0 Then ... 'Noncompliant always true
If array.Length >= 0 Then ... 'Noncompliant always true
For the sake of backward compatibility, Visual Basic .NET continues to offer a set of functions that convert from Object to different primitive types: CChar, CStr, CBool, CDate, CSng, CDbl, CDec, CByte, CSByte, CShort, CUShort, CInt, CUInt, CLng, CULng. However, using these functions is misleading, because it suggests a cast. It is better to cast explicitly using CType(), or use Convert.To() when the value should be converted.
Public Class Foo
Public Sub Bar(value as Object)
Dim stringValue As String = CStr(value)
End Sub
End Class
While it is technically correct to assign to parameters from within method bodies, doing so before the parameter value is read is likely a bug. Instead, initial values of parameters should be, if not treated as readonly then at least read before reassignment.
Module Module1
Sub Foo(ByVal a As Integer)
a = 42 ' Noncompliant
End Sub
End Module
This rule applies whenever an `If statement is followed by one or more ElseIf statements; the final ElseIf should be followed by an Else statement.
The requirement for a final Else statement is defensive programming.
The Else statement should either take appropriate action or contain a suitable comment as to why no action is taken. This is consistent with the requirement to have a final Case Else clause in a Select Case` statement.
If x = 0 Then
DoSomething()
ElseIf x = 1 Then
DoSomethingElse()
End If
Since Visual Studio 2010 SP1, the ByVal parameter modifier is implicitly applied, and therefore not required anymore. Removing it from your source code will improve readability.
Sub Foo(ByVal bar As String)
' ...
End Sub
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all even handler names match a provided regular expression.
The default configuration is:
Either in Pascal case, i.e. starting with an upper case letter, e.g. OnMyButtonClicked
Or, a subject, in Pascal or camel case, followed by an underscore followed by an event name, in Pascal case, e.g. btn1_Clicked
Event handlers with a handles clause and two-parameter methods with EventArgs second parameter are covered by this rule.
Module Module1
Sub subject__SomeEvent() Handles X.SomeEvent ' Noncompliant - two underscores
End Sub
End Module
Consistently using the `& operator for string concatenation make the developer intentions clear.
&, unlike +, will convert its operands to strings and perform an actual concatenation.
+` on the other hand can be an addition, or a concatenation, depending on the operand types.
Module Module1
Sub Main()
Console.WriteLine("1" + 2) ' Noncompliant - will display "3"
End Sub
End Module
Shared naming conventions allow teams to collaborate efficiently. This rule checks that all enum names match a provided regular expression.
The default configuration is the one recommended by Microsoft:
Pascal casing, starting with an upper case character, e.g. BackColor
Short abbreviations of 2 letters can be capitalized, e.g. GetID
Longer abbreviations need to be lower case, e.g. GetHtml
If the enum is marked as [Flags] then its name should be plural (e.g. MyOptions), otherwise, names should be singular (e.g. MyOption)
Public Enum foo ' Noncompliant
FooValue = 0
End Enum
Creating an extension method that extends Object is not recommended because it makes the method available on every type. Extensions should be applied at the most specialized level possible, and that is very unlikely to be Object.
Imports System.Runtime.CompilerServices
Module MyExtensions
<Extension>
Sub SomeExtension(obj As Object) ' Noncompliant
' ...
End Sub
End Module
An enumeration can be decorated with the FlagsAttribute to indicate that it can be used as a bit field: a set of flags, that can be independently set and reset.
For example, the following definition of the day of the week:
<Flags()>
Enum Days
Monday = 1 ' 0b00000001
Tuesday = 2 ' 0b00000010
Wednesday = 4 ' 0b00000100
Thursday = 8 ' 0b00001000
Friday = 16 ' 0b00010000
Saturday = 32 ' 0b00100000
Sunday = 64 ' 0b01000000
End Enum
The repetition of the Not operator is usually a typo. The second operator invalidates the first one:
Dim b As Boolean = False
Dim c As Boolean = Not Not b 'Noncompliant: equivalent to "b"
StringBuilder instances that never build a string clutter the code and worse are a drag on performance. Either they should be removed, or the missing ToString() call should be added.
Public Sub DoSomething(ByVal strings As List(Of String))
Dim sb As StringBuilder = New StringBuilder() ' Noncompliant
sb.Append("Got: ")
For Each str As String In strings
sb.Append(str).Append(", ")
Next
End Sub
To improve the code readability, the explicit line continuation character, _, should not be used. Instead, it is better to break lines after an operator.
Module Module1
Sub Main()
' Noncompliant
Console.WriteLine("Hello" _
& "world")
End Sub
End Module
Nested control flow statements If, Select, For, For Each, While, Do, and Try are often key ingredients in creating
what’s known as “Spaghetti code”. This code smell can make your program difficult to understand and maintain.
When numerous control structures are placed inside one another, the code becomes a tangled, complex web. This significantly reduces the code’s readability and maintainability, and it also complicates the testing process.
If condition1 ' Compliant - depth = 1
' ...
If condition2 ' Compliant - depth = 2
' ...
For i = 0 to 10 ' Compliant - depth = 3, not exceeding the limit
' ...
If condition4 ' Noncompliant - depth = 4
If condition5 ' Depth = 5, exceeding the limit, but issues are only reported on depth = 4
' ...
End If
Return
End If
Next
End If
End If
Recursion is a technique used to define a problem in terms of the problem itself, usually in terms of a simpler version of the problem itself.
For example, the implementation of the generator for the n-th value of the Fibonacci sequence comes naturally from its mathematical definition, when recursion is used:
Function NthFibonacciNumber(ByVal n As Integer) As Integer
If n <= 1 Then
Return 1
Else
Return NthFibonacciNumber(n - 1) + NthFibonacciNumber(n - 2)
End If
End Function
A Catch clause that only rethrows the caught exception has the same effect as omitting the Catch altogether and letting it bubble up automatically.
Dim s As String = ""
Try
s = File.ReadAllText(fileName)
Catch e As Exception
Throw
End Try
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all `Private Shared ReadOnly field names comply with the provided regular expression.
The default configuration is:
Optionally, can start with an underscore character or “s_”, e.g. _foo, s_foo`
Camel casing, starting with a lower case character, e.g. backColor
Short abbreviations of 2 letters can be capitalized only when not at the beginning, e.g. “id” in productID
Longer abbreviations need to be lower cased, e.g. html
Class Foo
Private Shared ReadOnly Foo As Integer ' Noncompliant
End Class
Nullable value types can hold either a value or Nothing.
The value stored in the nullable type can be accessed with the Value property or by casting it to the underlying type. Still, both operations throw an InvalidOperationException when the value is Nothing. A nullable type should always be tested before accessing the value to avoid raising exceptions.
Sub Sample(condition As Boolean)
Dim nullableValue As Integer? = If(condition, 42, Nothing)
Console.WriteLine(nullableValue.Value) ' Noncompliant: InvalidOperationException is raised
Dim nullableCast As Integer? = If(condition, 42, Nothing)
Console.WriteLine(CType(nullableCast, Integer)) ' Noncompliant: InvalidOperationException is raised
End Sub
Throwing general exceptions such as `Exception, SystemException and ApplicationException will have a negative impact on any code trying to catch these exceptions.
From a consumer perspective, it is generally a best practice to only catch exceptions you intend to handle. Other exceptions should ideally not be caught and let to propagate up the stack trace so that they can be dealt with appropriately. When a generic exception is thrown, it forces consumers to catch exceptions they do not intend to handle, which they then have to re-throw.
Besides, when working with a generic type of exception, the only way to distinguish between multiple exceptions is to check their message, which is error-prone and difficult to maintain. Legitimate exceptions may be unintentionally silenced and errors may be hidden.
For instance, if an exception such as StackOverflowException is caught and not re-thrown, it may prevent the program from terminating gracefully.
When throwing an exception, it is therefore recommended to throw the most specific exception possible so that it can be handled intentionally by consumers.
Additionally, some reserved exceptions should not be thrown manually. Exceptions such as IndexOutOfRangeException, NullReferenceException, OutOfMemoryException or ExecutionEngineException` will be thrown automatically by the runtime when the corresponding error occurs. Many of them indicate serious errors, which the application may not be able to recover from. It is therefore recommended to avoid throwing them as well as using them as base classes.
Public Sub DoSomething(obj As Object)
If obj Is Nothing Then
' Noncompliant
Throw New NullReferenceException("obj") ' Noncompliant: This reserved exception should not be thrown manually
End If
' ...
End Sub
Shared coding conventions allow teams to collaborate efficiently. This rule checks that property names match a provided regular expression.
The default configuration is the one recommended by Microsoft:
Pascal casing, starting with an upper case character, e.g. BackColor
Short abbreviations of 2 letters can be capitalized, e.g. GetID
Longer abbreviations need to be lower cased, e.g. GetHtml
Module Module1
Public Property foo As Integer ' Noncompliant
End Module
Properties provide a way to enforce encapsulation by providing property procedures that give controlled access to Private fields. However, in classes with multiple fields, it is not unusual that copy-and-paste is used to quickly create the needed properties, which can result in the wrong field being accessed by the property procedures.
Class C
Private _x As Integer
Private _Y As Integer
Public ReadOnly Property Y As Integer
Get
Return _x ' Noncompliant: The returned field should be '_y'
End Get
End Property
End Class
End statements exit the control flow of the program in an unstructured way. This statement stops code execution immediately without executing Dispose or Finalize methods, or executing Finally blocks. Therefore, it should be avoided.
Module Module1
Sub Print(ByVal str As String)
Try
...
End ' Noncompliant
Finally
' do something important here
...
End Try
End Sub
End Module
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all non-private fields names match a provided regular expression.
Note that this rule does not apply to non-private Shared ReadOnly fields, for which there is another rule.
The default configuration is:
Pascal casing, starting with an upper case character, e.g. BackColor
Short abbreviations of 2 letters can be capitalized, e.g. GetID
Longer abbreviations need to be lower cased, e.g. GetHtml
Class Foo
Public foo As Integer ' Noncompliant
End Class
Most checks against an IndexOf value compare it with -1 because 0 is a valid index.
strings.IndexOf(someString) = -1 ' Test for "index not found"
strings.IndexOf(someString) < 0 ' Test for "index not found"
strings.IndexOf(someString) >= 0 ' Test for "index found"
When Select Case statements have large sets of case clauses, it is usually an attempt to map two sets of data. A Dictionary should be used instead to make the code more readable and maintainable.
Public Class TooManyCase
Public Function MapValues(Ch As Char) As Integer
Select Case Ch ' Noncompliant: 5 cases, "Case Else" excluded, more than maximum = 4
Case "a"c
Return 1
Case "b"c, "c"c
Return 2
Case "d"c
Return 3
Case "e"c
Return 4
Case "f"c, "g"c, "h"c
Return 5
Case Else
Return 6
End Select
End Function
End Class
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
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all `Private Const field names comply with the provided regular expression.
The default configuration is:
Optionally, can start with an underscore character or “s_”, e.g. _foo, s_foo`
Camel casing, starting with a lower case character, e.g. backColor
Short abbreviations of 2 letters can be capitalized only when not at the beginning, e.g. “id” in productID
Longer abbreviations need to be lower cased, e.g. html
Module Module1
Private Const Foo = 0 ' Noncompliant
End Module
Declaring multiple variable on one line is difficult to read.
Module Module1
Public Const AAA As Integer = 5, BBB = 42, CCC As String = "foo" ' Noncompliant
End Module
Using .Count() to test for emptiness works, but using .Any() makes the intent clearer, and the code more readable. However, there are some cases where special attention should be paid:
if the collection is an EntityFramework or other ORM query, calling .Count() will cause executing a potentially massive SQL query and could put a large overhead on the application database. Calling .Any() will also connect to the database, but will generate much more efficient SQL.
if the collection is part of a LINQ query that contains .Select() statements that create objects, a large amount of memory could be unnecessarily allocated. Calling .Any() will be much more efficient because it will execute fewer iterations of the enumerable.
Private Function HasContent(Strings As IEnumerable(Of String)) As Boolean
Return Strings.Count() > 0 ' Noncompliant
End Function
Private Function HasContent2(Strings As IEnumerable(Of String)) As Boolean
Return Strings.Count() >= 1 ' Noncompliant
End Function
Private Function IsEmpty(Strings As IEnumerable(Of String)) As Boolean
Return Strings.Count() = 0 ' Noncompliant
End Function
The Select…Case 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 the case clause should be extracted into a dedicated procedure.
Select Case number
Case 1 To 5 ' Noncompliant: 4 statements in the case
MethodCall1("")
MethodCall2("")
MethodCall3("")
MethodCall4("")
Case Else
' ...
End Select
Shared naming conventions allow teams to collaborate efficiently. This rule checks that all subroutine and function names match a provided regular expression.
The default configuration is the one recommended by Microsoft:
Pascal casing, starting with an upper case character, e.g. BackColor
Short abbreviations of 2 letters can be capitalized, e.g. GetID
Longer abbreviations need to be lower cased, e.g. GetHtml
Event handlers with a handles clause and two-parameter methods with
EventArgssecond parameter are not covered by this rule.
Module Module1
Sub bad_subroutine() ' Noncompliant
End Sub
Public Function Bad_Function() As Integer ' Noncompliant
Return 42
End Function
End Module
Having all branches of a Select Case or If chain with the same implementation indicates a problem.
In the following code:
Dim b As Integer = If(a > 12, 4, 4) // Noncompliant
If b = 0 Then // Noncompliant
DoTheThing()
Else
DoTheThing()
End If
Select Case i // Noncompliant
Case 1
DoSomething()
Case 2
DoSomething()
Case 3
DoSomething()
Case Else
DoSomething()
End Select
Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate.
This rule allows to check that all interface names match a provided regular expression.
The default configuration is the one recommended by Microsoft:
Must start with an upper case ‘I’ character, e.g. IFoo
Followed by Pascal casing, starting with an upper case character, e.g. IEnumerable
Short abbreviations of 2 letters can be capitalized, e.g. IFooID
Longer abbreviations need to be lower cased, e.g. IFooHtml
Interface Foo ' Noncompliant
End Interface
A Do … Loop without a While or Until condition must be terminated by an unstructured Exit Do statement. It is safer and more readable to use structured loops instead.
Module Module1
Sub Main()
Dim i = 1
Do ' Non-Compliant
If i = 10 Then
Exit Do
End If
Console.WriteLine(i)
i = i + 1
Loop
End Sub
End Module
Inconsistent naming conventions can lead to confusion and errors when working in a team. This rule ensures that all generic type parameter names follow a consistent naming convention by checking them against a provided regular expression.
The default configuration follows Microsoft’s recommended convention:
Generic type parameter names must start with an upper case ‘T’, e.g. T
The rest of the name should use Pascal casing, starting with an upper case character, e.g. TKey
Short abbreviations of 2 letters can be capitalized, e.g. TFooID
Longer abbreviations should be lowercased, e.g. TFooHtml
Public Class Foo(Of tkey) ' Noncompliant
End Class
Methods declared as Public, Protected, or Protected Friend can be accessed from other assemblies, which means you should validate parameters to be within the expected constraints. In general, checking against Nothing is recommended in defensive programming.
This rule raises an issue when a parameter of a publicly accessible method is not validated against Nothing before being dereferenced.
Public Class Sample
Public Property Message As String
Public Sub PublicMethod(Arg As Exception)
Message = Arg.Message ' Noncompliant
End Sub
Protected Sub ProtectedMethod(Arg As Exception)
Message = Arg.Message ' Noncompliant
End Sub
End Class
There’s no need to null test in conjunction with an TypeOf … Is test. Nothing is not an instance of anything, so a null check is redundant.
If (x IsNot Nothing And TypeOf x Is MyClass)
' ...
End If
If (x Is Nothing Or TypeOf x IsNot MyClass)
' ...
End If
There is no good excuse for an empty class. If it’s being used simply as a common extension point, it should be replaced with an Interface. If it was stubbed in as a placeholder for future development it should be fleshed-out. In any other case, it should be eliminated.
Public Class Empty ' Noncompliant
End Class
Visual Basic .NET, unlike many other programming languages, has no “fall-through” for its Select cases. Each case already has an implicit Exit Select as its last instruction. It therefore is redundant to explicitly add one.
Module Module1
Sub Main()
Dim x = 0
Select Case x
Case 0
Console.WriteLine("0")
Exit Select ' Noncompliant
Case Else
Console.WriteLine("Not 0")
Exit Select ' Noncompliant
End Select
End Sub
End Module
Indexed properties are meant to represent access to a logical collection. When multiple parameters are required, this design guideline may be violated, and refactoring the property into a method is preferable.
Module Module1
ReadOnly Property Sum(ByVal a As Integer, ByVal b As Integer) ' Noncompliant
Get
Return a + b
End Get
End Property
End Module
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all enumeration value names match a provided regular expression.
The default configuration is the one recommended by Microsoft:
Pascal casing, starting with an upper case character, e.g. BackColor
Short abbreviations of 2 letters can be capitalized, e.g. GetID
Longer abbreviations need to be lower cased, e.g. GetHtml
Enum Foo
fooValue ' Noncompliant
End Enum
The … IsNot … syntax is more compact and more readable than the Not … Is … syntax.
Module Module1
Sub Main()
Dim a = Not "a" Is Nothing ' Noncompliant
End Sub
End Module
Prefer the use of `Try … Catch blocks instead of On Error statements.
Visual Basic .NET and Visual Basic 2005 offer structured exception handling that provides a powerful, more readable alternative to the On Error Goto error handling from previous versions of Microsoft Visual Basic. Structured exception handling is more powerful because it allows you to nest error handlers inside other error handlers within the same procedure. Furthermore, structured exception handling uses a block syntax similar to the If…Else…End If` statement. This makes Visual Basic .NET and Visual Basic 2005 code more readable and easier to maintain.
Sub DivideByZero()
On Error GoTo nextstep
Dim result As Integer
Dim num As Integer
num = 100
result = num / 0
nextstep:
System.Console.WriteLine("Error")
End Sub
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all `Private field names match the provided regular expression.
Note that this rule does not apply to Private Shared ReadOnly fields, which are checked by another rule.
The default configuration is:
Optionally, can start with an underscore character or “s_”, e.g. _foo, s_foo`
Camel casing, starting with a lower case character, e.g. backColor
Short abbreviations of 2 letters can be capitalized only when not at the beginning, e.g. “id” in productID
Longer abbreviations need to be lower cased, e.g. html
Class Foo
Private Foo As Integer ' Noncompliant
End Class
The … = {} syntax is more compact, more readable and less error-prone.
Module Module1
Sub Main()
Dim foo(1) As String ' Noncompliant
foo(0) = "foo"
foo(1) = "bar"
End Sub
End Module
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all namespace names match a provided regular expression.
The default configuration is the one recommended by Microsoft:
Pascal casing, starting with an upper case character, e.g. Microsoft, System
Short abbreviations of 2 letters can be capitalized, e.g. System.IO
Longer abbreviations need to be lower cased
Namespace foo ' Noncompliant
End Namespace
StringBuilder is more efficient than string concatenation, especially when the operator is repeated over and over as in loops.
Module Module1
Sub Main()
Dim foo = ""
foo &= "Result: " ' Compliant - outside of loop
For i = 1 To 9
foo &= i ' Noncompliant
Next
End Sub
End Module
Numbers are infinite, but the types that hold them are not. Each numeric type has hard upper and lower bounds. Try to calculate numbers beyond those bounds, and the result will be an OverflowException. When the compilation is configured to remove integer overflow checking, the value will be silently wrapped around from the expected positive value to a negative one, or vice versa.
Public Function Transform(Value As Integer) As Integer
If Value <= 0 Then Return Value
Dim Number As Integer = Integer.MaxValue
Return Number + Value ' Noncompliant
End Function
Method for creating empty arrays Array.Empty(Of TElement) was introduced in .NET 4.6 to optimize object instantiation and memory allocation. It also improves code readability by making developer’s intent more explicit. This new method should be preferred over empty array declaration.
Public Sub Method()
Dim Values1(-1) As Integer ' Noncompliant
Dim Values2 As Integer() = New Integer() {} ' Noncompliant
Dim Values3 As Integer() = {} ' Noncompliant
Dim Values4() As Integer = {} ' Noncompliant
End Sub
The requirement for a final Case Else clause is defensive programming.
This clause should either take appropriate action or contain a suitable comment as to why no action is taken.
Select Case param ' Noncompliant - Case Else clause is missing
Case 0
DoSomething()
Case 1
DoSomethingElse()
End Select
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
Subscribing to events without unsubscribing later on can lead to memory leaks or even duplicate subscriptions, i.e. code which is executed multiple times by mistake.
Even if there is no problem right now, the code is more difficult to review and a simple refactoring can create a bug. For example the lifetime of the event publisher could change and prevent subscribers from being garbage collected.
There are patterns to automatically unsubscribe, but the simplest and most readable solution remains to unsubscribe from events explicitly using `RemoveHandler.
This rule raises an issue when a class subscribes to an even using AddHandler without explicitly unsubscribing with RemoveHandler`.
Class MyEventProcucer
Public Shared Event EventFired As EventHandler
End Class
Public Class MyEventSubscriber
Implements IDisposable
Public Sub New()
AddHandler MyEventProcucer.EventFired, AddressOf c_EventFired 'Noncompliant
End Sub
Private Sub c_EventFired(sender As Object, e As EventArgs)
End Sub
Public Sub Dispose() Implements IDisposable.Dispose
End Sub
End Class
Passing parameters by reference requires developers to understand the subtle differences between reference and value types. It is preferable to avoid passing parameters by reference when possible.
Module Module1
Sub Foo(ByRef result As Integer) ' Non-Compliant
result = 42
End Sub
Sub Main()
Dim result As Integer
Foo(result)
Console.WriteLine(result)
End Sub
End Module
”After” and “Before” prefixes or suffixes should not be used to indicate pre and post events. The concepts of before and after should be given to events using the present and past tense.
Class Foo
Event BeforeClose() ' Noncompliant
Event AfterClose() ' Noncompliant
End Class
One of the principles of a unit test is that it must have full control of the system under test. This is problematic when production code includes calls to static methods, which cannot be changed or controlled. Date/time functions are usually provided by system libraries as static methods.
This can be improved by wrapping the system calls in an object or service that can be controlled inside the unit test.
Public Class Foo
Public Function HelloTime() As String
Return $"Hello at {DateTime.UtcNow}"
End Function
End Class
If a lock is acquired and released within a method, then it must be released along all execution paths of that method.
Failing to do so will expose the conditional locking logic to the method’s callers and hence be deadlock-prone.
Class Example
Private obj As Object = New Object()
Public Sub DoSomethingWithMonitor()
Monitor.Enter(obj) ' Noncompliant: not all paths release the lock
If IsInitialized() Then
' ...
Monitor.Exit(obj)
End If
End Sub
End Class
It is needlessly complex to invert the result of a boolean comparison. The opposite comparison should be made instead.
If Not (a = 2) Then // Noncompliant
Dim b as Boolean = Not (i < 10) // Noncompliant
Regular expressions have their own syntax that is understood by regular expression engines. Those engines will throw an exception at runtime if they are given a regular expression that does not conform to that syntax.
To avoid syntax errors, special characters should be escaped with backslashes when they are intended to be matched literally and references to capturing groups should use the correctly spelled name or number of the group.
Sub Regexes(Input As String)
Dim Rx As New Regex("[A") ' Noncompliant: unmatched "["
Dim Match = Regex.Match(Input, "[A") ' Noncompliant
Dim NegativeLookahead As New Regex("a(?!b)", RegexOptions.NonBacktracking) ' Noncompliant: negative lookahead without backtracking
Dim NegativeLookbehind As New Regex("(?<!a)b", RegexOptions.NonBacktracking) ' Noncompliant: negative lookbehind without backtracking
End Sub
Operating systems have global directories where any user has write access. Those folders are mostly used as temporary storage areas like `/tmp in Linux based systems. An application manipulating files from these folders is exposed to race conditions on filenames: a malicious user can try to create a file with a predictable name before the application does. A successful attack can result in other files being accessed, modified, corrupted or deleted. This risk is even higher if the application runs with elevated permissions.
In the past, it has led to the following vulnerabilities:
This rule raises an issue whenever it detects a hard-coded path to a publicly writable directory like /tmp (see examples bellow). It also detects access to environment variables that point to publicly writable directories, e.g., TMP and TMPDIR.
/tmp
/var/tmp
/usr/tmp
/dev/shm
/dev/mqueue
/run/lock
/var/run/lock
/Library/Caches
/Users/Shared
/private/tmp
/private/var/tmp
\Windows\Temp
\Temp
\TMP`
Dim RandomPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName())
' Creates a new file with write, non inheritable permissions which is deleted on close.
Using FileStream As New FileStream(RandomPath, FileMode.CreateNew, FileAccess.Write, FileShare.None, 4096, FileOptions.DeleteOnClose)
Using Writer As New StreamWriter(FileStream) ' Sensitive
' ...
End Using
End Using
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}[]
Dim a = 0 : Dim b = 0 ' Noncompliant
The information that an enumeration type is actually an enumeration or a set of flags should not be duplicated in its name.
Enum FooFlags ' Noncompliant
Foo = 1
Bar = 2
Baz = 4
End Enum
When executing an OS command and unless you specify the full path to the executable, then the locations in your application’s PATH environment variable will be searched for the executable. That search could leave an opening for an attacker if one of the elements in PATH is a directory under his control.
Dim p As New Process()
p.StartInfo.FileName = "C:\Apps\binary.exe" ' Compliant
Having a permissive Cross-Origin Resource Sharing policy is security-sensitive. It has led in the past to the following vulnerabilities:
Same origin policy in browsers prevents, by default and for security-reasons, a javascript frontend to perform a cross-origin HTTP request to a resource that has a different origin (domain, protocol, or port) from its own. The requested target can append additional HTTP headers in response, called CORS, that act like directives for the browser and change the access control policy / relax the same origin policy.
Response.AppendHeader("Access-Control-Allow-Origin", "trustedwebsite.com") // Compliant
towns.Item(x) = "London"
towns.Item(x) = "Chicago"; // Noncompliant
When declaring a Windows Communication Foundation (WCF) OperationContract method as one-way, that service method won’t return any result, not even an underlying empty confirmation message. These are fire-and-forget methods that are useful in event-like communication. Therefore, specifying a return type has no effect and can confuse readers.
<ServiceContract>
Interface IMyService
<OperationContract(IsOneWay:=True)>
Function SomethingHappened(ByVal parameter As Integer) As Integer ' Noncompliant
End Interface
Development tools and frameworks usually have options to make debugging easier for developers. Although these features are useful during development, they should never be enabled for applications deployed in production. Debug instructions or error messages can leak detailed information about the system, like the application’s path or file names.
Imports Microsoft.AspNetCore.Builder
Imports Microsoft.AspNetCore.Hosting
Namespace MyMvcApp
Public Class Startup
Public Sub Configure(ByVal app As IApplicationBuilder, ByVal env As IHostingEnvironment)
' Those calls are Sensitive because it seems that they will run in production
app.UseDeveloperExceptionPage() 'Sensitive
app.UseDatabaseErrorPage() 'Sensitive
End Sub
End Class
End Namespace
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 BooleanMethod() = True Then ' Noncompliant
' ...
End If
If BooleanMethod() = False Then ' Noncompliant
' ...
End If
If BooleanMethod() OrElse False Then ' Noncompliant
' ...
End If
DoSomething(Not False) ' Noncompliant
DoSomething(BooleanMethod() = True) ' Noncompliant
Dim booleanVariable = If(BooleanMethod(), True, False) ' Noncompliant
booleanVariable = If(BooleanMethod(), True, exp) ' Noncompliant
booleanVariable = If(BooleanMethod(), False, exp) ' Noncompliant
booleanVariable = If(BooleanMethod(), exp, True) ' Noncompliant
booleanVariable = If(BooleanMethod(), exp, False) ' Noncompliant
This rule is meant to be used as a way to track code which is marked as being deprecated. Deprecated code should eventually be removed.
<Obsolete> ' Noncompliant
Sub Procedure()
End Sub
Properties with only setters are confusing and counterintuitive. Instead, a property getter should be added if possible, or the property should be replaced with a setter method.
Module Module1
WriteOnly Property Foo() As Integer ' Non-Compliant
Set(ByVal value As Integer)
' ... some code ...
End Set
End Property
End Module
When optional parameter values are not passed to base method calls, the value passed in by the caller is ignored. This can cause the function to behave differently than expected, leading to errors and making the code difficult to debug.
Public Class BaseClass
Public Overridable Sub MyMethod(ByVal Optional i As Integer = 1)
Console.WriteLine(i)
End Sub
End Class
Public Class DerivedClass
Inherits BaseClass
Public Overrides Sub MyMethod(ByVal Optional i As Integer = 1)
' ...
MyBase.MyMethod() ' Noncompliant: caller's value is ignored
End Sub
Private Shared Function Main(ByVal args As String()) As Integer
Dim dc As DerivedClass = New DerivedClass()
dc.MyMethod(12) ' prints 1
End Function
End Class
Fields should not be part of an API, and therefore should always be private. Indeed, they cannot be added to an interface for instance, and validation cannot be added later on without breaking backward compatibility. Instead, developers should encapsulate their fields into properties. Explicit property getters and setters can be introduced for validation purposes or to smooth the transition to a newer system.
Class Foo
Public Foo = 42 ' Noncompliant
End Class
Successful Zip Bomb attacks occur when an application expands untrusted archive files without controlling the size of the expanded data, which can lead to denial of service. A Zip bomb is usually a malicious archive file of a few kilobytes of compressed data but turned into gigabytes of uncompressed data. To achieve this extreme compression ratio, attackers will compress irrelevant data (eg: a long string of repeated bytes).
Const ThresholdRatio As Double = 10
Const ThresholdSize As Integer = 1024 * 1024 * 1024 ' 1 GB
Const ThresholdEntries As Integer = 10000
Dim TotalSizeArchive, TotalEntryArchive, TotalEntrySize, Cnt As Integer
Dim Buffer(1023) As Byte
Using ZipToOpen As New FileStream("ZipBomb.zip", FileMode.Open), Archive As New ZipArchive(ZipToOpen, ZipArchiveMode.Read)
For Each Entry As ZipArchiveEntry In Archive.Entries
Using s As Stream = Entry.Open
TotalEntryArchive += 1
TotalEntrySize = 0
Do
Cnt = s.Read(Buffer, 0, Buffer.Length)
TotalEntrySize += Cnt
TotalSizeArchive += Cnt
If TotalEntrySize / Entry.CompressedLength > ThresholdRatio Then Exit Do ' Ratio between compressed And uncompressed data Is highly suspicious, looks Like a Zip Bomb Attack
Loop While Cnt > 0
End Using
If TotalSizeArchive > ThresholdSize Then Exit For ' The uncompressed data size Is too much for the application resource capacity
If TotalEntryArchive > ThresholdEntries Then Exit For ' Too much entries in this archive, can lead to inodes exhaustion of the system
Next
End Using
Having a field in a child class with a name that differs from a parent class’ field only by capitalization is sure to cause confusion. Such child class fields should be renamed.
Public Class Fruit
Protected PlantingSeason As String
' ...
End Class
Public Class Raspberry
Inherits Fruit
Protected Plantingseason As String ' Noncompliant
' ...
End Class
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 condition1 Then
If condition2 Then ' Noncompliant
' ...
End If
End If
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
Clear-text protocols such as `ftp, telnet, or http lack encryption of transported data, as well as the capability to build an authenticated connection. It means that an attacker able to sniff traffic from the network can read, modify, or corrupt the transported content. These protocols are not secure as they expose applications to an extensive range of risks:
sensitive data exposure
traffic redirected to a malicious endpoint
malware-infected software update or installer
execution of client-side code
corruption of critical information
Even in the context of isolated networks like offline environments or segmented cloud environments, the insider threat exists. Thus, attacks involving communications being sniffed or tampered with can still happen.
For example, attackers could successfully compromise prior security layers by:
bypassing isolation mechanisms
compromising a component of the network
getting the credentials of an internal IAM account (either from a service account or an actual person)
In such cases, encrypting communications would decrease the chances of attackers to successfully leak data or steal credentials from other network components. By layering various security practices (segmentation and encryption, for example), the application will follow the defense-in-depth principle.
Note that using the http` protocol is being deprecated by major web browsers.
In the past, it has led to the following vulnerabilities:
Dim UrlHttp As String = "http://example.com" ' Noncompliant
Dim UrlFtp As String = "ftp://anonymous@example.com" ' Noncompliant
Dim UrlTelnet As String = "telnet://anonymous@example.com" ' Noncompliant
The use of parentheses, even those not required to enforce a desired order of operations, can clarify the intent behind a piece of code. However, redundant pairs of parentheses could be misleading and should be removed.
If a AndAlso ((x + y > 0)) Then ' Noncompliant
' ...
End If
Return ((x + 1)) ' Noncompliant
Array designators should always be located on the type for better code readability. Otherwise, developers must look both at the type and the variable name to know whether or not a variable is an array.
Module Module1
Sub Main()
Dim foo() As String ' Noncompliant
End Sub
End Module
Arbitrary OS command injection vulnerabilities are more likely when a shell is spawned rather than a new process, indeed shell meta-chars can be used (when parameters are user-controlled for instance) to inject OS commands.
Imports System.Security
Imports System.Diagnostics
Namespace N
Class A
Public Sub Foo(ByVal process As Process)
Process.Start("/usr/bin/file.exe") ' Compliant
End Sub
End Class
End Namespace
Rejecting requests with significant content length is a good practice to control the network traffic intensity and thus resource consumption in order to prevent DoS attacks.
Imports Microsoft.AspNetCore.Mvc
Public Class MyController
Inherits Controller
<HttpPost>
<DisableRequestSizeLimit> ' Sensitive: No size limit
<RequestSizeLimit(10485760)> ' Sensitive: 10485760 B = 10240 KB = 10 MB is more than the recommended limit of 8MB
Public Function PostRequest(Model model) As IActionResult
' ...
End Function
<HttpPost>
<RequestFormLimits(MultipartBodyLengthLimit = 10485760)> ' Sensitive: 10485760 B = 10240 KB = 10 MB is more than the recommended limit of 8MB
Public Function MultipartFormRequest(Model model) As IActionResult
' ...
End Function
End Class
Formatted SQL queries can be difficult to maintain, debug and can increase the risk of SQL injection when concatenating untrusted values into the query. However, this rule doesn’t detect SQL injections (unlike rule S3649), the goal is only to highlight complex/formatted queries.
Public Sub Foo(ByVal context As DbContext, ByVal value As String)
context.Database.ExecuteSqlCommand("SELECT * FROM mytable WHERE mycol=@p0", value) ' Compliant, the query is parameterized
End Sub
A typical code smell known as unused function parameters refers to parameters declared in a function but not used anywhere within the function’s body. While this might seem harmless at first glance, it can lead to confusion and potential errors in your code. Disregarding the values passed to such parameters, the function’s behavior will be the same, but the programmer’s intention won’t be clearly expressed anymore. Therefore, removing function parameters that are not being utilized is considered best practice.
Private Sub DoSomething(ByVal a As Integer, ByVal b as Integer) ' "b" is unused
Compute(a)
End Sub
Private Function DoSomething2(ByVal a As Integer, ByVal b As Integer) As Integer ' "a" is unused
Compute(b)
Return b
End Function
Shared naming conventions allow teams to collaborate efficiently.
This rule raises an issue when a class name does not match a provided regular expression.
Class foo ' Noncompliant
End Class
Assemblies should explicitly indicate whether they are meant to be COM visible or not. If the ComVisibleAttribute is not present, the default is to make the content of the assembly visible to COM clients.
Note that COM visibility can be overridden for individual types and members.
Namespace MyLibrary ' Noncompliant
End Namespace
switch 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 variable
Case 0
doSomething()
Case Else
doSomethingElse()
End Select
Conditional expressions which are always true or false can lead to unreachable code.
Dim a = False
If a Then
Dispose() ' Never reached
End If
There is no reason to re-assign a variable to itself. Either this statement is redundant and should be removed, or the re-assignment is a mistake and some other value or variable was intended for the assignment instead.
Public Sub SetName(name As String)
name = name ' Noncompliant
End Sub
Serialization event handlers that don’t have the correct signature will not be called, bypassing augmentations to automated serialization and deserialization events.
A method is designated a serialization event handler by applying one of the following serialization event attributes:
Serialization event handlers take a single parameter of type StreamingContext, return void, and have private visibility.
This rule raises an issue when any of these constraints are not respected.
<Serializable>
Public Class Foo
<OnSerializing>
Public Sub OnSerializing(ByVal context As StreamingContext) ' Noncompliant: should be private
End Sub
<OnSerialized>
Private Function OnSerialized(ByVal context As StreamingContext) As Integer ' Noncompliant: should return void
End Function
<OnDeserializing>
Private Sub OnDeserializing() ' Noncompliant: should have a single parameter of type StreamingContext
End Sub
<OnSerializing>
Public Sub OnSerializing2(Of T)(ByVal context As StreamingContext) ' Noncompliant: should have no type parameters
End Sub
<OnDeserialized>
Private Sub OnDeserialized(ByVal context As StreamingContext, ByVal str As String) ' Noncompliant: should have a single parameter of type StreamingContext
End Sub
End Class
Assemblies should conform with the Common Language Specification (CLS) in order to be usable across programming languages. To be compliant an assembly has to indicate it with System.CLSCompliantAttribute.
<Assembly: CLSCompliant(True)>
Namespace MyLibrary
End Namespace
Constant members are copied at compile time to the call sites, instead of being fetched at runtime.
As an example, say you have a library with a constant `Version member set to 1.0, and a client application linked to it. This library is then updated and Version is set to 2.0. Unfortunately, even after the old DLL is replaced by the new one, Version will still be 1.0 for the client application. In order to see 2.0, the client application would need to be rebuilt against the new version of the library.
This means that you should use constants to hold values that by definition will never change, such as Zero`. In practice, those cases are uncommon, and therefore it is generally better to avoid constant members.
This rule only reports issues on public constant fields, which can be reached from outside the defining assembly.
Public Class Foo
Public Const Version = 1.0 ' Noncompliant
End Class
According to the Single Responsibility Principle, introduced by Robert C. Martin in his book “Principles of Object Oriented Design”, a class should have only one responsibility:
Classes which rely on many other classes tend to aggregate too many responsibilities and should be split into several smaller ones.
Nested classes dependencies are not counted as dependencies of the outer class.
Class Foo ' Non-Compliant - depends on too many other classes
Dim a1 As T1 ' Foo is coupled to T1
Dim a2 As T2 ' Foo is coupled to T2
Dim a3 As T3 ' Foo is coupled to T3
Dim a4 As T4 ' etc.
Dim a5 As T5
Dim a6 As T6
Dim a7 As T7
Dim a8 As T8
Dim a9 As T9
Dim a10 As T10
Dim a11 As T11
Dim a12 As T12
Dim a13 As T13
Dim a14 As T14
Dim a15 As T15
Dim a16 As T16
Dim a17 As T17
Dim a18 As T18
Dim a19 As T19
Dim a20 As T20
Dim a21 As T21
End Class
Nested ternaries are hard to read and can make the order of operations complex to understand.
Unresolved directive in <stdin> - include::{noncompliant}[]
Instead, use another line to express the nested operation in a separate statement.
Unresolved directive in <stdin> - include::{compliant}[]
Public Function GetReadableStatus(job As Job) As String
Return If(job.IsRunning, "Running", If(job.HasErrors, "Failed", "Succeeded")) ' Noncompliant
End Function
Clear, communicative naming is important in code. It helps maintainers and API users understand the intentions for and uses of a unit of code. Using “exception” in the name of a class that does not extend Exception or one of its subclasses is a clear violation of the expectation that a class’ name will indicate what it is and/or does.
Public Class FruitException ' Noncompliant - this has nothing to do with Exception
Private expected As Fruit
Private unusualCharacteristics As String
Private appropriateForCommercialExploitation As Boolean
' ...
End Class
Public Class CarException ' Noncompliant - does not derive from any Exception-based class
Public Sub New(message As String, inner As Exception)
' ...
End Sub
End Class
For clarity, all overloads of the same method should be grouped together. That lets both users and maintainers quickly understand all the current available options.
Interface IMyInterface
Function DoTheThing() As Integer
Function DoTheOtherThing() As String // Noncompliant
Function DoTheThing(ByVal Path As String) As Integer
End Interface
Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all occurrences.
Public Class Foo
Private Name As String = "foobar" ' Noncompliant
Public ReadOnly Property DefaultName As String = "foobar" ' Noncompliant
Public Sub New(Optional Value As String = "foobar") ' Noncompliant
Dim Something = If(Value, "foobar") ' Noncompliant
End Sub
End Class
Types are declared in namespaces in order to prevent name collisions and as a way to organize them into the object hierarchy. Types that are defined outside any named namespace are in a global namespace that cannot be referenced in code.
Public Class Foo
End Class
Public Structure Bar
End Structure
Altering or bypassing the accessibility of classes, methods, or fields through reflection violates the encapsulation principle. This can break the internal contracts of the accessed target and lead to maintainability issues and runtime errors.
This rule raises an issue when reflection is used to change the visibility of a class, method or field, and when it is used to directly update a field value.
Imports System.Reflection
Dim dynClass = Type.GetType("MyInternalClass")
' Sensitive. Using BindingFlags.NonPublic will return non-public members
Dim bindingAttr As BindingFlags = BindingFlags.NonPublic Or BindingFlags.Static
Dim dynMethod As MethodInfo = dynClass.GetMethod("mymethod", bindingAttr)
Dim result = dynMethod.Invoke(dynClass, Nothing)
CoSetProxyBlanket and CoInitializeSecurity both work to set the permissions context in which the process invoked immediately after is executed. Calling them from within that process is useless because it’s too late at that point; the permissions context has already been set.
Specifically, these methods are meant to be called from non-managed code such as a C++ wrapper that then invokes the managed, i.e. C# or VB.NET, code.
Public Class Noncompliant
<DllImport("ole32.dll")>
Public Shared Function CoSetProxyBlanket(<MarshalAs(UnmanagedType.IUnknown)>pProxy As Object, dwAuthnSvc as UInt32, dwAuthzSvc As UInt32, <MarshalAs(UnmanagedType.LPWStr)> pServerPrincName As String, dwAuthnLevel As UInt32, dwImpLevel As UInt32, pAuthInfo As IntPtr, dwCapabilities As UInt32) As Integer
End Function
Public Enum RpcAuthnLevel
[Default] = 0
None = 1
Connect = 2
[Call] = 3
Pkt = 4
PktIntegrity = 5
PktPrivacy = 6
End Enum
Public Enum RpcImpLevel
[Default] = 0
Anonymous = 1
Identify = 2
Impersonate = 3
[Delegate] = 4
End Enum
Public Enum EoAuthnCap
None = &H00
MutualAuth = &H01
StaticCloaking = &H20
DynamicCloaking = &H40
AnyAuthority = &H80
MakeFullSIC = &H100
[Default] = &H800
SecureRefs = &H02
AccessControl = &H04
AppID = &H08
Dynamic = &H10
RequireFullSIC = &H200
AutoImpersonate = &H400
NoCustomMarshal = &H2000
DisableAAA = &H1000
End Enum
<DllImport("ole32.dll")>
Public Shared Function CoInitializeSecurity(pVoid As IntPtr, cAuthSvc As Integer, asAuthSvc As IntPtr, pReserved1 As IntPtr, level As RpcAuthnLevel, impers As RpcImpLevel, pAuthList As IntPtr, dwCapabilities As EoAuthnCap, pReserved3 As IntPtr) As Integer
End Function
Public Sub DoSomething()
Dim Hres1 As Integer = CoSetProxyBlanket(Nothing, 0, 0, Nothing, 0, 0, IntPtr.Zero, 0) ' Noncompliant
Dim Hres2 As Integer = CoInitializeSecurity(IntPtr.Zero, -1, IntPtr.Zero, IntPtr.Zero, RpcAuthnLevel.None, RpcImpLevel.Impersonate, IntPtr.Zero, EoAuthnCap.None, IntPtr.Zero) ' Noncompliant
End Sub
End Class
Not specifying a timeout for regular expressions can lead to a Denial-of-Service attack. Pass a timeout when using System.Text.RegularExpressions to process untrusted input because a malicious user might craft a value for which the evaluation lasts excessively long.
Ask Yourself Whether
the input passed to the regular expression is untrusted.
the regular expression contains patterns vulnerable to catastrophic backtracking.
There is a risk if you answered yes to any of those questions.
Recommended Secure Coding Practices
It is recommended to specify a matchTimeout when executing a regular expression.
Make sure regular expressions are not vulnerable to Denial-of-Service attacks by reviewing the patterns.
Consider using a non-backtracking algorithm by specifying RegexOptions.NonBacktracking.
Public Sub RegexPattern(Input As String)
Dim EmailPattern As New Regex(".+@.+", RegexOptions.None)
Dim IsNumber as Boolean = Regex.IsMatch(Input, "[0-9]+")
Dim IsLetterA as Boolean = Regex.IsMatch(Input, "(a+)+")
End Sub
ASP.NET 1.1+ comes with a feature called Request Validation, preventing the server to accept content containing un-encoded HTML. This feature comes as a first protection layer against Cross-Site Scripting (XSS) attacks and act as a simple Web Application Firewall (WAF) rejecting requests potentially containing malicious content.
While this feature is not a silver bullet to prevent all XSS attacks, it helps to catch basic ones. It will for example prevent <script type=“text/javascript” src=“https://malicious.domain/payload.js”> to reach your Controller.
Note: Request Validation feature being only available for ASP.NET, no Security Hotspot is raised on ASP.NET Core applications.
<ValidateInput(True)>
Public Function Welcome(Name As String) As ActionResult
...
End Function
The overloading mechanism should be used in place of optional parameters for several reasons:
Optional parameter values are baked into the method call site code, thus, if a default value has been changed, all referencing assemblies need to be rebuilt, otherwise the original values will be used.
The Common Language Specification (CLS) allows compilers to ignore default parameter values, and thus require the caller to explicitly specify the values. For example, if you want to consume a method with default argument from another .NET compatible language (for instance C++/CLI), you will have to provide all arguments. When using method overloads, you could achieve similar behavior as default arguments.
Optional parameters prevent muddying the definition of the function contract. Here is a simple example: if there are two optional parameters, when one is defined, is the second one still optional or mandatory?
Sub Notify(ByVal Company As String, Optional ByVal Office As String = "QJZ") ' Noncompliant
End Sub
To customize the default behavior for an export in the Managed Extensibility Framework (MEF), applying the PartCreationPolicyAttribute is necessary. For the PartCreationPolicyAttribute to be meaningful in the context of an export, the class must also be annotated with the ExportAttribute.
This rule raises an issue when a class is annotated with the PartCreationPolicyAttribute but not with the ExportAttribute.
Imports System.ComponentModel.Composition
<PartCreationPolicy(CreationPolicy.Any)> ' Noncompliant
Public Class FooBar
Inherits IFooBar
End Class
{upper_function}s with a long parameter list are difficult to use because maintainers must figure out the role of each parameter and keep track of their position.
Unresolved directive in <stdin> - include::{language}/noncompliant.adoc[]
The solution can be to:
Split the {function} into smaller ones
Unresolved directive in <stdin> - include::{language}/split-example.adoc[]
Find a better data structure for the parameters that group data in a way that makes sense for the specific application domain
Unresolved directive in <stdin> - include::{language}/struct-example.adoc[]
This rule raises an issue when a {function} has more parameters than the provided threshold.
Class BaseClass
Sub New(Param1 As Integer)
' ...
End Sub
End Class
Class DerivedClass
Inherits BaseClass
Public Sub New(Param1 As Integer, Param2 As Integer, Param3 As Integer, Param4 As Integer, Param5 As Long)
' Compliant by exception: Param1 is used in the base class constructor, so it does not count in the sum of the parameter list.
MyBase.New(Param1)
' ...
End Sub
End Class
Two {func_name}s having the same implementation are suspicious. It might be that something else was intended. Or the duplication is intentional, which becomes a maintenance burden.
Private Const CODE As String = "bounteous"
Private callCount As Integer = 0
Public Function GetCode() As String
callCount = callCount + 1
Return CODE
End Function
Public Function GetName() As String ' Noncompliant: duplicates GetCode
callCount = callCount + 1
Return CODE
End Function
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.
Dim ip = ConfigurationManager.AppSettings("myapplication.ip") ' Compliant
Dim address = IPAddress.Parse(ip)
When the syntax new Guid() (i.e. parameterless instantiation) is used, it must be that one of three things is wanted:
An empty GUID, in which case Guid.Empty is clearer.
A randomly-generated GUID, in which case Guid.NewGuid() should be used.
A new GUID with a specific initialization, in which case the initialization parameter is missing.
This rule raises an issue when a parameterless instantiation of the Guid struct is found.
Public Sub Foo()
Dim G1 As New Guid ' Noncompliant - what's the intent?
Dim G2 As Guid = Nothing ' Noncompliant
End Sub
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.
Sub DoSomething()
' TODO
End Sub
ViewBag and ViewData dictionaries enable controllers to pass data to their views as weakly typed collections. Reading the provided values is dynamically resolved at runtime without any compile-time checking. This can lead to unexpected behavior, since reading a missing value does not produce an exception.
Controllers should pass data to their views via a strongly typed view model class.
Public Class HomeController
Inherits Controller
Public Function Article() As ActionResult
ViewBag.Title = "Title" ' Noncompliant, model should be used
ViewData("Text") = "Text" ' Noncompliant, model should be used
Return View()
End Function
End Class
The ExcludeFromCodeCoverageAttribute is used to exclude portions of code from code coverage reporting. It is a bad practice to retain code that is not covered by unit tests. In .Net 5, the Justification property was added to the ExcludeFromCodeCoverageAttribute as an opportunity to document the rationale for the exclusion. This rule raises an issue when no such justification is given.
Public Structure Coordinates
Public ReadOnly Property X As Integer
Public ReadOnly Property Y As Integer
<ExcludeFromCodeCoverage> ' Noncompliant
Public Overrides Function Equals(obj As Object) As Boolean
If Not (TypeOf obj Is Coordinates) Then
Return False
End If
Dim coordinates = DirectCast(obj, Coordinates)
Return X = coordinates.X AndAlso
Y = coordinates.Y
End Function
<ExcludeFromCodeCoverage> ' Noncompliant
Public Overrides Function GetHashCode() As Integer
Dim hashCode As Long = 1861411795
hashCode = (hashCode * -1521134295 + X.GetHashCode()).GetHashCode()
hashCode = (hashCode * -1521134295 + Y.GetHashCode()).GetHashCode()
Return hashCode
End Function
End Structure
When you annotate an Enum with the Flags attribute, you must not rely on the values that are automatically set by the language to the Enum members, but you should define the enumeration constants in powers of two (1, 2, 4, 8, and so on). Automatic value initialization will set the first member to zero and increment the value by one for each subsequent member. As a result, you won’t be able to use the enum members with bitwise operators.
<Flags()>
Enum FruitType ' Non-Compliant
None
Banana
Orange
Strawberry
End Enum
Module Module1
Sub Main()
Dim bananaAndStrawberry = FruitType.Banana Or FruitType.Strawberry
Console.WriteLine(bananaAndStrawberry.ToString()) ' Will display only "Strawberry"
End Sub
End Module
In Unix file system permissions, the “others” category refers to all
users except the owner of the file system resource and the members of the group
assigned to this resource.
Granting permissions to this category can lead to unintended access to files or directories that could allow attackers to obtain sensitive information, disrupt services or elevate privileges.
Dim safeAccessRule = new FileSystemAccessRule("Everyone", FileSystemRights.FullControl, AccessControlType.Deny)
Dim fileSecurity = File.GetAccessControl("path")
fileSecurity.AddAccessRule(safeAccessRule)
File.SetAccessControl("path", fileSecurity)
`if statements with conditions that are always false have the effect of making blocks of code non-functional. if statements with conditions that are always true are completely redundant, and make the code less readable.
There are three possible causes for the presence of such code:
An if statement was changed during debugging and that debug code has been committed.
Some value was left unset.
Some logic is not doing what the programmer thought it did.
In any of these cases, unconditional if` statements should be removed.
If True Then ' Noncompliant
DoSomething()
End If
If False Then ' Noncompliant
DoSomethingElse()
End If
Having a variable with the same name in two unrelated classes is fine, but do the same thing within a class hierarchy and you’ll get confusion at best, chaos at worst.
Public Class Fruit
Protected Ripe As Season
Protected Flesh As Color
' ...
End Class
Public Class Raspberry
Inherits Fruit
Private Ripe As Boolean ' Noncompliant
Private Shared FLESH As Color ' Noncompliant
' ...
End Class
Cryptographic hash algorithms such as MD2, MD4, MD5, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160 and SHA-1 are no longer considered secure, because it is possible to have collisions (little computational effort is enough to find two or more different inputs that produce the same hash).
Imports System.Security.Cryptography
Sub ComputeHash()
Dim sha256 = New SHA256CryptoServiceProvider() ' Compliant
Dim sha384 = New SHA384CryptoServiceProvider() ' Compliant
Dim sha512 = New SHA512CryptoServiceProvider() ' Compliant
' ...
End Sub
It may be a good idea to raise an exception in a constructor if you’re unable to fully flesh the object in question, but not in an exception constructor. If you do, you’ll interfere with the exception that was originally being thrown. Further, it is highly unlikely that an exception raised in the creation of an exception will be properly handled in the calling code, and the unexpected, unhandled exception will lead to program termination.
Class MyException
Inherits Exception
Public Sub MyException()
If bad_thing Then
Throw New Exception("A bad thing happened")
End If
End Sub
End Class
Fields marked with `System.Runtime.Serialization.OptionalFieldAttribute are serialized just like any other field. But such fields are ignored on deserialization, and retain the default values associated with their types. Therefore, deserialization event handlers should be declared to set such fields during the deserialization process.
This rule raises when at least one field with the System.Runtime.Serialization.OptionalFieldAttribute attribute is declared but one (or both) of the following event handlers System.Runtime.Serialization.OnDeserializingAttribute or System.Runtime.Serialization.OnDeserializedAttribute` are not present.
<Serializable>
Public Class Foo ' Noncompliant
<OptionalField(VersionAdded:=2)>
Private optionalField As Integer = 5
End Class
The use of a non-standard algorithm is dangerous because a determined attacker may be able to break the algorithm and compromise whatever data has been protected. Standard algorithms like `SHA-256, SHA-384, SHA-512, … should be used instead.
This rule tracks creation of java.security.MessageDigest` subclasses.
Dim mySHA256 As SHA256 = SHA256.Create()
Because it is easy to extract strings from an application source code or binary, credentials should not be hard-coded. This is particularly true for applications that are distributed or that are open-source.
In the past, it has led to the following vulnerabilities:
Credentials should be stored outside of the code in a configuration file, a database, or a management service for secrets.
This rule flags instances of hard-coded credentials used in database and LDAP connections. It looks for hard-coded credentials in connection strings, and for variable names that match any of the patterns from the provided list.
It’s recommended to customize the configuration of this rule with additional credential words such as “oauthToken”, “secret”, …
Dim username As String = "admin"
Dim password As String = GetEncryptedPassword()
Dim usernamePassword As String = String.Format("user={0}&password={1}", GetEncryptedUsername(), GetEncryptedPassword())
Dim url As String = $"scheme://{username}:{password}@domain.com"
Dim url2 As String= "http://guest:guest@domain.com" ' Compliant
Const Password_Property As String = "custom.password" ' Compliant
There are valid cases for passing a variable multiple times into the same method call, but usually doing so is a mistake, and something else was intended for one of the arguments.
If compare(myPoint.x, myPoint.x) <> 0 Then ' Noncompliant
'...
End If
If compare(getNextValue(), getNextValue()) <> 0 Then ' Noncompliant
'...
End If
Marking a method with the Pure attribute indicates that the method doesn’t make any visible state changes. Therefore, a Pure method should return a result. Otherwise, it indicates a no-operation call.
Using Pure on a void method is either by mistake or the method is not doing a meaningful task.
Class Person
Private age As Integer
<Pure> ' Noncompliant: The method makes a state change
Private Sub ConfigureAge(ByVal age As Integer)
Me.age = age
End Sub
<Pure>
Private Sub WriteAge() ' Noncompliant
Console.WriteLine(Me.age)
End Sub
End Class
It should be clear to a casual reader what code a test is testing and what results are expected. Unfortunately, that’s not usually the case with the ExpectedException attribute since an exception could be thrown from almost any line in the method.
This rule detects MSTest and NUnit ExpectedException attribute.
<TestMethod>
<ExpectedException(GetType(ArgumentNullException))> ' Noncompliant
Public Sub TestNullArg()
'...
End Sub
An unused local variable is a variable that has been declared but is not used anywhere in the block of code where it is defined. It is dead code, contributing to unnecessary complexity and leading to confusion when reading the code. Therefore, it should be removed from your code to maintain clarity and efficiency.
Using t = New TestTimer()
End Using
Disposing an object twice in the same method, either with the {usingArg} or by calling Dispose directly, is confusing and error-prone. For example, another developer might try to use an already-disposed object, or there can be runtime errors for specific paths in the code.
In addition, even if the documentation explicitly states that calling the Dispose method multiple times should not throw an exception, some implementations still do it. Thus it is safer to not dispose of an object twice when possible.
Dim foo As New Disposable()
foo.Dispose()
foo.Dispose() ' Noncompliant
If an exception is already being thrown within the try block or caught in a catch block, throwing another exception in the finally block will override the original exception. This means that the original exception’s message and stack trace will be lost, potentially making it challenging to diagnose and troubleshoot the root cause of the problem.
Try
' Some work which end up throwing an exception
Throw New ArgumentException()
Finally
' Cleanup
Throw New InvalidOperationException() ' Noncompliant: will mask the ArgumentException
End Try
When creating a custom Markup Extension that accepts parameters in WPF, the ConstructorArgument markup must be used to identify the discrete properties that match these parameters. However since this is done via a string, the compiler won’t give you any warning in case there are typos.
This rule raises an issue when the string argument to ConstructorArgumentAttribute doesn’t match any parameter of any constructor.
Imports System
Namespace MyLibrary
Public Class MyExtension
Inherits MarkupExtension
Public Sub New()
End Sub
Public Sub New(ByVal value1 As Object)
Value1 = value1
End Sub
<ConstructorArgument("value2")> ' Noncompliant
Public Property Value1 As Object
End Class
End Namespace
Cognitive Complexity is a measure of how hard it is to understand the control flow of a unit of code. Code with high cognitive complexity is hard to read, understand, test, and modify.
As a rule of thumb, high cognitive complexity is a sign that the code should be refactored into smaller, easier-to-manage pieces.
Function Abs(ByVal n As Integer) As Integer ' Noncompliant: cognitive complexity = 5
If n >= 0 Then ' +1
Return n
Else ' +2, due to nesting
If n = Integer.MinValue Then ' +1
Throw New ArgumentException("The absolute value of int.MinValue is outside of int boundaries")
Else ' +1
Return -n
End If
End If
End Function
The AssemblyVersion attribute is used to specify the version number of an assembly. An assembly is a compiled unit of code, which can be marked with a version number by applying the attribute to an assembly’s source code file.
The AssemblyVersion attribute is useful for many reasons:
Versioning: The attribute allows developers to track and manage different versions of an assembly. By incrementing the version number for each new release, you can easily identify and differentiate between different versions of the same assembly. This is particularly useful when distributing and deploying software, as it helps manage updates and compatibility between different versions.
Dependency management: When an assembly references another assembly, it can specify the specific version of the dependency it requires. By using the AssemblyVersion attribute, you can ensure that the correct version of the referenced assembly is used. This helps avoid compatibility issues and ensures that the expected behavior and functionality are maintained.
GAC management: The GAC, also known as Global Assembly Cache, is a central repository for storing shared assemblies on a system. The AssemblyVersion attribute plays a crucial role in managing assemblies in the GAC. Different versions of an assembly can coexist in the GAC, allowing applications to use the specific version they require.
If no AssemblyVersion is provided, the same default version will be used for every build. Since the version number is used by .NET Framework to uniquely identify an assembly, this can lead to broken dependencies.
Imports System.Reflection
<Assembly: AssemblyTitle("MyAssembly")> ' Noncompliant
Namespace MyLibrary
' ...
End Namespace
The Attributed Programming Model, also known as Attribute-oriented programming (@OP), is a programming model used to embed attributes within codes.
In this model, objects are required to conform to a specific structure so that they can be used by the Managed Extensibility Framework (MEF).
MEF provides a way to discover available components implicitly, via composition. A MEF component, called a part, declaratively specifies:
both its dependencies, known as imports
and what capabilities it makes available, known as exports
The ExportAttribute declares that a part “exports”, or provides to the composition container, an object that fulfills a particular contract.
During composition, parts with imports that have matching contracts will have those dependencies filled by the exported object.
If the type doesn’t implement the interface it is exporting there will be an issue at runtime (either a cast exception or just a container not filled with the exported type) leading to unexpected behaviors/crashes.
The rule raises an issue when a class doesn’t implement or inherit the type declared in the ExportAttribute.
<Export(GetType(ISomeType))>
Public Class SomeType ' Noncompliant: doesn't implement 'ISomeType'.
End Class
Control flow constructs like if-statements allow the programmer to direct the flow of a program depending on a boolean expression. However, if the condition is always true or always false, only one of the branches will ever be executed. In that case, the control flow construct and the condition no longer serve a purpose; they become gratuitous.
Dim d As String = Nothing
Dim v1 = If(d, "value")
A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes. {identifier_capital_plural} hold the meaning of the written code. Their names should be meaningful and follow a consistent and easily recognizable pattern. Adhering to a consistent naming convention helps to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.
This rule checks that {identifier} names match a provided regular expression.
Module Module1
Sub Main()
Dim Foo = 0 ' Noncompliant
End Sub
End Module
Date and time should not be used as a type for primary keys
Public Class Account
Public Property Id As DateTime
Public Property Name As String
Public Property Surname As String
End Class
Exception types should be “Public"
Friend Class MyException ' Noncompliant
Inherits Exception
' ...
End Class
"Obsolete” attributes should include explanations
Public Class Car
<Obsolete> ' Noncompliant
Public Sub CrankEngine(Turns As Integer)
' ...
End Sub
End Class
Having two Cases in the same Select statement or branches in the same If structure with the same implementation is at best duplicate code, and at worst a coding error.
If a >= 0 AndAlso a < 10 Then
DoFirst()
DoTheThing()
ElseIf a >= 10 AndAlso a < 20 Then
DoTheOtherThing()
ElseIf a >= 20 AndAlso a < 50 ' Noncompliant; duplicates first condition
DoFirst()
DoTheThing()
Else
DoTheRest();
End If
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.