# We checked the source code of VS Code No more tiptoeing, we can finally say it out loud: we've released a static code analyzer for JavaScript and TypeScript\. So naturally, it's time to send it out into the wild and see what it catches in the source code of Visual Studio Code, an open\-source project most developers know all too well\. Here's what turned up\. ![1407_vscode_check/image1.jpeg](https://import.viva64.com/docx/blog/1407_vscode_check/image1.jpeg) ## Before we start This section is for anyone who isn't familiar with our article format, so you know what to expect\. At PVS\-Studio, we build a [static code analysis](https://pvs-studio.com/en/blog/terms/0046/) tool\. Basically, a static analyzer is a program that looks for logical errors in the source code of other programs\. For this kind of article, we pick a popular open\-source project, run our analyzer on it, and share [what we find](https://pvs-studio.com/en/blog/inspections/) with our readers, as well as with the project's developers, by filing issues and pull requests\. With the current release, we're adding a new analyzer for JavaScript and TypeScript\. Right now it runs on a basic set of diagnostic rules, but it's already capable of catching real errors and flagging suspicious code\. To show it tackling something real, we picked a project pretty much everyone's heard of, [Visual Studio Code](https://code.visualstudio.com/)\. It combines JavaScript and TypeScript code, and the codebase is quite large, so we were curious to see what our newcomer would find in it\. We ran the analysis on [release version 1\.128\.0](https://github.com/microsoft/vscode/releases/tag/1.128.0), the latest version at the time of writing, at commit [fc3def677](https://github.com/microsoft/vscode/tree/fc3def6774c76082adf699d366f31a557ce5573f)\. We cloned the repo, ran the analysis, and once it finished, picked out the most interesting results\. Now for the results\. ## Accessing an array in a strange way In this section, and in the ones that follow, you'll see a code snippet containing either an outright error or code that looks off and is worth a second look, followed by the PVS\-Studio warning and a link to the source in the repository\. Right after that, we'll explore what's wrong with the actual code\. The first error for today is a rather extravagant way to access an array: ```cpp .... const diffs = changes.map(splice => { return [splice[0], splice[1], splice[2].map(....)] as [number, number, CellViewModel[]]; }); .... for (let i = 0; i < diffs.length; i++) { const diff = diffs[0]; // <= if (diff[0] + diff[1] <= primarySelectionIndex) { delta += diff[2].length - diff[1]; continue; } if (diff[0] > primarySelectionIndex) { endSelectionHandles = [primaryHandle]; break; } if (diff[0] + diff[1] > primarySelectionIndex) { endSelectionHandles = [this._viewCells[diff[0] + delta].handle]; break; } } .... ``` **The PVS\-Studio warning**: [V7016](https://pvs-studio.com/en/docs/warnings/v7016/) Suspicious access to an element of the 'diffs' object by a constant index inside a loop\. [notebookViewModelImpl\.ts 252](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts#L252) Here, we have an array being built, followed by an attempt to loop through it\. Why an attempt? The `i` index is never actually used within the array, instead, the line marked with the comment `// <=` always pulls the element at index 0\. As a result, on every single iteration, checking `diff` means dealing with the first element of the `diffs` collection\. Judging by the snippet itself, that wasn't the intent\. The plan was clearly to check each `i`\-th element, but a typo is a typo\. This is a recurring pattern across other languages too\. Our Java analyzer, for instance, has caught [similar issues](https://pvs-studio.com/en/blog/examples/v6016/) before\. So we're moving on\. ## Checking the wrong thing ```cpp .... let lastValueAtPosition: boolean | undefined = undefined; let lastValueOnLine: boolean | undefined = undefined; timeouts.add(autorun(reader => { const newValueAtPosition = s.source.isPresentAtPosition(args.position, reader); const newValueOnLine = s.source.isPresentOnLine(args.position.lineNumber, reader); if ( lastValueAtPosition !== undefined && lastValueAtPosition !== undefined) { // <= if (!lastValueAtPosition && newValueAtPosition) { trigger(s.property, s.source, 'positional'); } if (!lastValueOnLine && newValueOnLine) { trigger(s.property, s.source, 'line'); } } lastValueAtPosition = newValueAtPosition; lastValueOnLine = newValueOnLine; })); .... ``` **The PVS\-Studio warning**: [V7001](https://pvs-studio.com/en/docs/warnings/v7001/) The operands of the '&&' operator in the 'lastValueAtPosition \!\=\= undefined && lastValueAtPosition \!\=\= undefined' expression are equivalent\. [editorTextPropertySignalsContribution\.ts 152](https://github.com/microsoft/vscode/blob/6caa618a9e0feed9a1025bbb4914a3a8eb1695d7/src/vs/workbench/contrib/accessibilitySignals/browser/editorTextPropertySignalsContribution.ts#L152) The code has two variables, `lastValueAtPosition` and `lastValueOnLine`, both typed as `boolean` or `undefined`\. And in the arrow function they're passed into, the intent was to first check that neither one is `undefined`, and only then treat them as `boolean` values\. There's a typo in the condition the analyzer flagged: the binary expression `&&` uses identical operands\. As a result, `lastValueOnLine` never gets checked against `undefined`, meaning the second nested `if` statement can behave incorrectly\. ## Shady self\-assignment The assignment operator comes with its share of sneaky typos, here's the first one\. **The first snippet**: ```cpp swapChildren(from: number, to: number): void { from = validateIndex(from, this.children.length); to = validateIndex(to, this.children.length); if (from === to) { return; } this.splitview.swapViews(from, to); // swap boundary sashes [this.children[from].boundarySashes, this.children[to].boundarySashes] = [this.children[from].boundarySashes, this.children[to].boundarySashes]; // <= // swap children [this.children[from], this.children[to]] = [this.children[to], this.children[from]]; this.onDidChildrenChange(); } ``` **The PVS\-Studio warning**: [V7005](https://pvs-studio.com/en/docs/warnings/v7005/) The expression '\[this\.children\[from\]\.boundarySashes, this\.children\[to\]\.boundarySashes\]' is assigned to itself\. [gridview\.ts 575](https://github.com/microsoft/vscode/blob/6caa618a9e0feed9a1025bbb4914a3a8eb1695d7/src/vs/base/browser/ui/grid/gridview.ts#L575) In this code, the goal was to use [destructuring](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring) to swap the values of `this.children[from].boundarySashes` and `this.children[to].boundarySashes`\. Destructuring is a handy way to do this kind of swap without introducing a third variable\. But for it to actually work, the values on the right side of the assignment needed to be in the opposite order\. As it stands, the `// swap boundary sashes` comment doesn't match what the code actually does\. By the way, a similar swap happens [further down](https://github.com/microsoft/vscode/blob/6caa618a9e0feed9a1025bbb4914a3a8eb1695d7/src/vs/base/browser/ui/grid/gridview.ts#L579), with different values, and that one doesn't have this typo\. **The second snippet:** ```cpp export class LanguageModelTextPart implements vscode.LanguageModelTextPart2 { value: string; audience: vscode.LanguageModelPartAudience[] | undefined; constructor(value: string, audience?: vscode.LanguageModelPartAudience[]) { this.value = value; audience = audience; // <= } toJSON() { return { $mid: MarshalledId.LanguageModelTextPart, value: this.value, audience: this.audience, }; } } ``` **The PVS\-Studio warning**: [V7005](https://pvs-studio.com/en/docs/warnings/v7005/) The variable 'audience' is assigned to itself\. [extHostTypes\.ts 4033](https://github.com/microsoft/vscode/blob/6caa618a9e0feed9a1025bbb4914a3a8eb1695d7/src/vs/workbench/api/common/extHostTypes.ts#L4033) Another assignment typo, but in a different context this time\. There's a constructor parameter named `audience` and a class field also named `audience`\. When assigning a value to the `audience` field, the code forgot the `this` keyword\. As a result, the `audience` field never gets initialized\. **The third snippet**: ```cpp export class ServerInstalledExtensionsView extends ExtensionsListView { override async show(query: string): Promise> { query = query ? query : '@installed'; if (....) { query = query += ' @installed'; // <= } return super.show(query.trim()); } } ``` **The PVS\-Studio warning**: [V7005](https://pvs-studio.com/en/docs/warnings/v7005/) The variable 'query' is assigned to itself\. [extensionsViews\.ts 1310](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/contrib/extensions/browser/extensionsViews.ts#L1310) This one's similar, but it's hardly an error, more of a typo or an odd use of the `+=` operator\. The `+=` operator already assigns the new value to `query` on its own, so the `query = ....` part in front of it is redundant\. In this particular spot, it's very unlikely to cause any real trouble\. But the same kind of typo, if it landed on a plain `=` assignment meant for a different variable, could actually cause problems\. And beyond that, a new developer reading this code would have to stop and figure out whether there's a real bug here or whether the redundant assignment is just harmless\. That's reason enough not to leave code like this as is\. There're similar warnings in the project: * V7005 The variable 'filePath' is assigned to itself\. [fileLoading\.ts 31](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/extensions/copilot/src/extension/inlineEdits/test/node/fileLoading.ts#L31) * V7005 The expression 'tunnelProperties\.privacy' is assigned to itself\. [tunnelModel\.ts 773](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/services/remote/common/tunnelModel.ts#L773) * V7005 The variable 'value' is assigned to itself\. [task\.ts 75](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/test/automation/src/task.ts#L75) ## HTTPS or HTTPS? ```cpp private onFocusChanged(event: ITableEvent) { if (event.indexes.length > 0 && event.elements.length > 0) { this.lastFocus = [...event.indexes]; } const elements = event.elements; const item = elements && elements.length ? elements[0] : undefined; if (item) { this.tunnelViewSelectionContext.set( makeAddress(item.remoteHost, item.remotePort)); this.tunnelTypeContext.set(item.tunnelType); this.tunnelCloseableContext.set(!!item.closeable); this.tunnelPrivacyContext.set(item.privacy.id); this.tunnelProtocolContext.set(item.protocol === TunnelProtocol.Https ? TunnelProtocol.Https : TunnelProtocol.Https); // <= .... } } ``` **The PVS\-Studio warning**: [V7012](https://pvs-studio.com/en/docs/warnings/v7012/) \[CWE\-1041\] The conditional expression 'item\.protocol \=\=\= TunnelProtocol\.Https ? TunnelProtocol\.Https : TunnelProtocol\.Https' always returns the same value\. [tunnelView\.ts 997](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/contrib/remote/browser/tunnelView.ts#L997) Here, the analyzer flags that both branches of the ternary return the same value, `TunnelProtocol.Https`\. We checked `TunnelProtocol`, and sure enough, the `else` branch should be returning `TunnelProtocol.Http` instead\. Interestingly, we'd already flagged this one ourselves, and while double\-checking it before writing this article, we noticed that a few days after our check, one of VS Code's users had spotted the same bug independently\. Here's a [link](https://github.com/microsoft/vscode/pull/325445) to their pull request\. This is a good example of static analysis catching errors before they make it into the master branch\. Another warning for [V7012](https://pvs-studio.com/en/docs/warnings/v7012/) also flagged the **following snippet**: ```cpp .... if (this.selection.endColumn <= this.targetPosition.column) { // The target position is after the selection's end position this.targetSelection = new Selection( this.targetPosition.lineNumber - this.selection.endLineNumber + this.selection.startLineNumber, this.selection.startLineNumber === this.selection.endLineNumber ? this.targetPosition.column - this.selection.endColumn + this.selection.startColumn : this.targetPosition.column - this.selection.endColumn + this.selection.startColumn, // <= this.targetPosition.lineNumber, this.selection.startLineNumber === this.selection.endLineNumber ? this.targetPosition.column : this.selection.endColumn ); } .... ``` **The PVS\-Studio warning**: [V7012](https://pvs-studio.com/en/docs/warnings/v7012/) \[CWE\-1041\] The conditional expression always returns the same value\. [dragAndDropCommand\.ts 86](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/editor/contrib/dnd/browser/dragAndDropCommand.ts#L86) Here too, the analyzer points out identical expressions in the branches of the ternary operator, but this one isn't as clear\-cut as the last\. It looks like the `then` and `else` branches probably aren't meant to be identical, but what one of them should actually contain is anyone's guess\. This is a case where the fix should come from someone who fully understands what this code is meant to do\. ## Unreachable condition ```cpp .... const lineCount = model.getLineCount(); const endLine = lineNumber === lineCount; const prevLineEmptyOrIndented = lineNumber > 1 && isLineEmptyOrIndented(lineNumber - 1); const nextLineEmptyOrIndented = !endLine && isLineEmptyOrIndented(lineNumber + 1); const currLineEmptyOrIndented = isLineEmptyOrIndented(lineNumber); const notEmpty = !nextLineEmptyOrIndented && !prevLineEmptyOrIndented; // check above and below. if both are blocked, display lightbulb in the gutter. if (!nextLineEmptyOrIndented && !prevLineEmptyOrIndented && !hasDecoration) { this._gutterState.set(....); this.renderGutterLightbub(); return this.hide(); } else if (prevLineEmptyOrIndented || endLine || (prevLineEmptyOrIndented && !currLineEmptyOrIndented)) { // <= effectiveLineNumber -= 1; } .... ``` **The PVS\-Studio warning:** [V7018](https://pvs-studio.com/en/docs/warnings/v7018/) The expression 'prevLineEmptyOrIndented \|\| \(prevLineEmptyOrIndented && \.\.\.\)' is redundant and always evaluates to 'prevLineEmptyOrIndented'\. [lightBulbWidget\.ts 373](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/editor/contrib/codeAction/browser/lightBulbWidget.ts#L373) In this snippet, the analyzer caught an odd pattern in the condition\. The warning itself points out that if you set aside the second subcondition, `|| endLine`, what's left is `prevLineEmptyOrIndented || (prevLineEmptyOrIndented && ...)`\. Once the condition is reduced to that form, it's clear that we'll either never reach the second part, or it won't evaluate to `true` anyway\. ## Forgotten return ```cpp .... if (focusedRepository) { .... // Resource Groups const resourceGroups: string[] = []; for (const resourceGroup of focusedRepository.provider.groups) { resourceGroups.push( `${resourceGroup.label} (${resourceGroup.resources.length} resource(s))`); } focusedRepository.provider.groups.map(g => g.label).join(', '); // <= content.push( localize( 'state-msg6', "Resource groups: {0}", resourceGroups.join(', '))); } .... ``` **The PVS\-Studio warning:** [V7010](https://pvs-studio.com/en/docs/warnings/v7010/) \[CWE\-252\] The return value of function 'join' is required to be utilized\. [scmAccessibilityHelp\.ts 108](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/contrib/scm/browser/scmAccessibilityHelp.ts#L108) The problem is that the result of calling [`join`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/join) is never stored anywhere\. Because of that, the line calling it is doing literally nothing\. Bugs like this usually come from one of two things: either a developer just forgets to write the assignment, or there's confusion about how a method actually works, whether it mutates the object in place or returns a new value\. Since the code above shows [the correct usage](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/contrib/scm/browser/scmAccessibilityHelp.ts#L75) of `join`, we're leaning toward the first explanation\. ## Identical operands **The first snippet:** ```cpp .... if (gapOriginalLength > 0) { const gapStartOffset = nesOffset + lastChange.originalStart + lastChange.originalLength; const gapStartPos = textModel.getPositionAt(gapStartOffset); const wordRange = textModel.getWordAtPosition(gapStartPos); if (wordRange) { const wordStartOffset = textModel.getOffsetAt( new Position(gapStartPos.lineNumber, wordRange.startColumn)); const wordEndOffset = textModel.getOffsetAt( new Position(gapStartPos.lineNumber, wordRange.endColumn)); const gapEndOffset = gapStartOffset + gapOriginalLength; if (wordStartOffset <= gapStartOffset && gapEndOffset <= wordEndOffset && wordStartOffset <= gapEndOffset && gapEndOffset <= wordEndOffset) { // <= lastChange.originalLength = (change.originalStart + change.originalLength) - lastChange.originalStart; lastChange.modifiedLength = (change.modifiedStart + change.modifiedLength) - lastChange.modifiedStart; continue; } } } .... ``` **The PVS\-Studio warning**: [V7001](https://pvs-studio.com/en/docs/warnings/v7001/) The operands of the '&&' operator are equivalent\. [renameSymbolProcessor\.ts 141](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/editor/contrib/inlineCompletions/browser/model/renameSymbolProcessor.ts#L141) Here, the analyzer is pointing at this condition: ```cpp wordStartOffset <= gapStartOffset && gapEndOffset <= wordEndOffset && wordStartOffset <= gapEndOffset && gapEndOffset <= wordEndOffset ``` Since the variable names in this condition are so similar to each other, the issue isn't obvious at a glance\. But look closely and `gapEndOffset <= wordEndOffset` is checked twice in that chain\. That repetition is what makes this potentially risky, since this could be more than just a harmless extra condition\. When code has a run of similar\-looking lines, variable names, or object names, it's easy to accidentally reference the wrong one\. That's exactly the kind of mistake a quick read\-through can miss, since the operands of `&&` all look alike here\. Hopefully it really is just an unnecessary condition, nothing more\. **The second snippet:** ```cpp .... if ( response.status === 401 && text.includes('authorize_url') && jsonData?.authorize_url ) { return { type: FetchResponseKind.Failed, modelRequestId: modelRequestIdObj, failKind: ChatFailKind.AgentUnauthorized, reason: response.statusText || response.statusText, // <= data: jsonData }; } .... ``` **The PVS\-Studio warning:** [V7001](https://pvs-studio.com/en/docs/warnings/v7001/) The operands of the '\|\|' operator in the 'response\.statusText \|\| response\.statusText' expression are equivalent\. [chatMLFetcher\.ts 1641](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts#L1641) Another one like the last, and it looks like a typo too\. Elsewhere in the code, `reason` is set as follows: ```cpp reason: jsonData.message || 'Invalid previous response ID' ``` Or like this: ```cpp reason: jsonData?.message || `token expired or invalid: ${response.status}` ``` **The third snippet:** ```cpp .... // Notebooks are not supported yet. if (URI.isUri(variableRef.value)) { if (await this.ignoreService.isCopilotIgnored(variableRef.value)) { return; } if ( variableRef.value.scheme === Schemas.vscodeNotebookCellOutput || variableRef.value.scheme === Schemas.vscodeNotebookCellOutput ) { return; } .... validReferences.push(variableRef); fileFolderReferences.push(variableRef); return; } .... ``` **The PVS\-Studio warning:** [V7001](https://pvs-studio.com/en/docs/warnings/v7001/) The operands of the '\|\|' operator are equivalent\. [copilotcliPromptResolver\.ts 136](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliPromptResolver.ts#L136) The `variableRef.value.scheme` field is compared against the same constant from `Schemas`\. Here are the other candidates in `Schemas` that this comparison could have been checking against instead: ```cpp export const vscodeNotebookCell = 'vscode-notebook-cell'; export const vscodeNotebookCellMetadata = 'vscode-notebook-cell-metadata'; export const vscodeNotebookCellMetadataDiff = 'vscode-notebook-cell-metadata-diff'; export const vscodeNotebookCellOutput = 'vscode-notebook-cell-output'; export const vscodeNotebookCellOutputDiff = 'vscode-notebook-cell-output-diff'; export const vscodeNotebookMetadata = 'vscode-notebook-metadata'; export const vscodeInteractiveInput = 'vscode-interactive-input'; ``` **The fourth snippet:** ```cpp static isEqual(a: Diagnostic | undefined, b: Diagnostic | undefined): boolean { if (a === b) { return true; } if (!a || !b) { return false; } return a.message === b.message && a.severity === b.severity && a.code === b.code && a.severity === b.severity && a.source === b.source && a.range.isEqual(b.range) && equals(a.tags, b.tags) && equals(a.relatedInformation, b.relatedInformation, DiagnosticRelatedInformation.isEqual); } ``` **The PVS\-Studio warning**: [V7001](https://pvs-studio.com/en/docs/warnings/v7001/) The operands of the '&&' operator are equivalent\. [diagnostic\.ts 99](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/api/common/extHostTypes/diagnostic.ts#L99) The expression `a.severity === b.severity` shows up twice here\. It looks odd, but every field of `Diagnostic` is already checked, so the developers just tacked on a redundant condition\. The exact same thing happens in this next comparison: [V7001](https://pvs-studio.com/en/docs/warnings/v7001/) The operands of the '&&' operator are equivalent\. [keyboardLayoutService\.ts 431](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/services/keybinding/browser/keyboardLayoutService.ts#L431)
To wrap up, a bit more on this group of V7001 warnings\\\. Every example here comes down to the same question: is the repeated operand just redundant, or is it a real typo? Figuring that out usually eats up more time than it should, which is exactly why this kind of thing shouldn't slip into the code in the first place\. But mistakes like this are genuinely easy to miss during a code review\. Unlike a person, a static analyzer never loses focus, which makes it invaluable when working through large amounts of repetitive code\. Static analysis pays off in plenty of other situations too, but these examples make its benefits especially clear\.
## Misleading formatting ```cpp if (isResourceMergeEditorInput(editor)) { .... } if (isResourceDiffEditorInput(editor)) { .... } else if (isResourceEditorInput(editor)) { resources.add(editor.resource); } ``` **The PVS\-Studio warning:** [V7030](https://pvs-studio.com/en/docs/warnings/v7030/) \[CWE\-670\] Suspicious code formatting\. The 'else' keyword is probably missing\. [editorService\.ts 739](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/services/editor/browser/editorService.ts#L739) Above is a shortened snippet, with the original formatting kept intact\. Judging by the formatting and the pattern of checks, that second block should be an `else if`, just like the last one\. We looked closely at this snippet and didn't find anything worse than an extra check running when the first condition is met\. But code like this: 1. is misleading; 1. runs an unnecessary check; 1. and if the final statement were a plain `else` instead of `else if`, it would run even after the first `if` had already executed\. That last point is the scary one, and it's a good thing this particular block uses `else if`\. Just like before, this is the kind of thing a human reviewer can easily miss, but a static analyzer catches it without any trouble\. ## When static analysis prepares the ground for refactoring Besides catching potential errors, as we covered above, a static analyzer can also be a real asset for refactoring\. A high number of warnings in one area can be a sign that certain code could, and sometimes should, be simplified\. That's what we'll look at next\. ### Simplify switch\-case ```cpp function walkChildren( node: TSESTree.Node, visit: (child: TSESTree.Node) => void ) { switch (node.type) { .... case 'ForInStatement': // <= visit(node.left); visit(node.right); visit(node.body); break; case 'ForOfStatement': visit(node.left); visit(node.right); visit(node.body); break; case 'WhileStatement': case 'DoWhileStatement': visit(node.test); visit(node.body); break; .... default: break; } } ``` **The PVS\-Studio warning**: [V7008](https://pvs-studio.com/en/docs/warnings/v7008/) \[CWE\-691\] Two or more case branches perform the same actions\. [code\-no\-accessor\-after\-await\.ts 365](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/.eslint-plugin-local/code-no-accessor-after-await.ts#L365) In this snippet, the `case` branches for `ForInStatement` and `ForOfStatement` do exactly the same thing\. Given that `WhileStatement` and `DoWhileStatement` are already [stacked just below](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/.eslint-plugin-local/code-no-accessor-after-await.ts#L375), there's a good argument for doing the same here\. ### Some occasional self\-subtraction ```cpp .... while (byteCount > 0) { const chunk = this._chunks[chunkIndex]; if (chunk.byteLength > byteCount) { // this chunk will survive const chunkPart = chunk.slice(0, byteCount); result.set(chunkPart, resultOffset); resultOffset += byteCount; if (advance) { this._chunks[chunkIndex] = chunk.slice(byteCount); this._totalLength -= byteCount; } byteCount -= byteCount; // <= } else { // this chunk will be entirely read .... byteCount -= chunk.byteLength; } } return result; .... ``` **The PVS\-Studio warning**: [V7014](https://pvs-studio.com/en/docs/warnings/v7014/) The identical expression 'byteCount' to the left and to the right of a compound assignment\. [ipc\.net\.ts 243](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/base/parts/ipc/common/ipc.net.ts#L243) This one made it into the refactoring section because it's very likely the developers actually meant to zero out `byteCount` here, to guarantee the loop would exit\. Figuring that out took a moment, though, since the `else` branch just below makes it look like this could just as easily be a typo\. To make the intent clear, it's better to just set the value to `0` directly\. ### Simplify the condition: ```cpp set(element: TextEditElement) { this._localDisposables.clear(); this._localDisposables.add( dom.addDisposableListener(this._checkbox, 'change', e => { element.setChecked(this._checkbox.checked); e.preventDefault(); })); if (element.parent.isChecked()) { this._checkbox.checked = element.isChecked(); this._checkbox.disabled = element.isDisabled(); } else { this._checkbox.checked = element.isChecked(); this._checkbox.disabled = element.isDisabled(); } .... } ``` **The PVS\-Studio warning**: [V7004](https://pvs-studio.com/en/docs/warnings/v7004/) \[CWE\-691\] The 'then' statement is equivalent to the 'else' statement\. [bulkEditTree\.ts 595](https://github.com/microsoft/vscode/blob/fc3def6774c76082adf699d366f31a557ce5573f/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditTree.ts#L595) As things stand, the `else` block in this snippet doesn't do anything meaningful\. But like all the other snippets here, this one is best left for VS Code's active contributors and maintainers to look into\. ## Before we go And that brings us to the end of the article\. We've walked you through what we think are the most interesting things the PVS\-Studio analyzer found in the VS Code source code\. That's a solid outcome for us: even though the JavaScript and TypeScript analyzer is only just launching as an MVP, it's already catching suspicious code and real errors, even if a lot of them are simple typos for now\. Down the line, once we add [data\-flow](https://pvs-studio.com/en/blog/terms/7004/) analysis and other features, the analyzer will get better at spotting more complex issues too\. By the way, some of our diagnostic rules started out as ideas our users and blog readers [suggested](https://pvs-studio.com/en/blog/posts/java/1185/) to us\. So if you've got thoughts on what kinds of errors our analyzer should be catching in JavaScript and TypeScript code, we'd love to hear them, some might even become real rules\. It was a lot of fun writing this article for you, and we hope you enjoyed reading it\. Good luck out there, and see you next time\. P\.S\. You can try our analyzer on your own project [here](https://pvs-studio.com/en/pvs-studio/try-free/)\.