diff --git a/code-fixtures/fixer-tests/MyClass2.cls b/code-fixtures/fixer-tests/MyClass2.cls new file mode 100644 index 0000000..3a9182c --- /dev/null +++ b/code-fixtures/fixer-tests/MyClass2.cls @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2023, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: BSD-3-Clause + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ +public class MyClass2 { + + public static boolean someBooleanMethod() { + // some comment that includes public class MyClass2 { + return false; + } + /* some other comment in a single line */ + public static boolean someOtherBooleanMethod() { + /* + some other comment that includes public class MyClass 2 { + */ + return false; + } + + public static boolean someOtherMethod() { + public static String someString = 'this string has \' class MyClass2 { '; + return true; + } + + private class MyInnerClass { + // Some inner class + } +} diff --git a/src/extension.ts b/src/extension.ts index 256352e..66404b7 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -85,9 +85,9 @@ export async function activate(context: vscode.ExtensionContext): Promise { + const removeDiagnosticsOnSelectedFile = vscode.commands.registerCommand(Constants.COMMAND_REMOVE_DIAGNOSTICS_ON_SELECTED_FILE, async (selection: vscode.Uri, multiSelect?: vscode.Uri[]) => { return _clearDiagnosticsForSelectedFiles(multiSelect && multiSelect.length > 0 ? multiSelect : [selection], { - commandName: Constants.COMMAND_REMOVE_DIAGNOSTRICS_ON_SELECTED_FILE, + commandName: Constants.COMMAND_REMOVE_DIAGNOSTICS_ON_SELECTED_FILE, diagnosticCollection, outputChannel }); diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 9ee6bda..868b5e2 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -14,7 +14,7 @@ export const COMMAND_RUN_ON_ACTIVE_FILE = 'sfca.runOnActiveFile'; export const COMMAND_RUN_ON_SELECTED = 'sfca.runOnSelected'; export const COMMAND_RUN_DFA_ON_SELECTED_METHOD = 'sfca.runDfaOnSelectedMethod'; export const COMMAND_REMOVE_DIAGNOSTICS_ON_ACTIVE_FILE = 'sfca.removeDiagnosticsOnActiveFile'; -export const COMMAND_REMOVE_DIAGNOSTRICS_ON_SELECTED_FILE = 'sfca.removeDiagnosticsOnSelectedFile'; +export const COMMAND_REMOVE_DIAGNOSTICS_ON_SELECTED_FILE = 'sfca.removeDiagnosticsOnSelectedFile'; export const COMMAND_DIAGNOSTICS_IN_RANGE = 'sfca.removeDiagnosticsInRange' diff --git a/src/lib/fixer.ts b/src/lib/fixer.ts index 03a3910..9dc7dbb 100644 --- a/src/lib/fixer.ts +++ b/src/lib/fixer.ts @@ -92,6 +92,12 @@ export class _NoOpFixGenerator extends FixGenerator { * @private Must be exported for testing purposes, but shouldn't be used publicly, hence the leading underscore. */ export class _PmdFixGenerator extends FixGenerator { + public singleLineCommentPattern = /^\s*\/\//; + public blockCommentStartPattern = /^\s*\/\*/; + public blockCommentEndPattern = /\*\//; + public classDeclarationPattern = /\b(\w+\s+)+class\s+\w+/; + public suppressionRegex = /@SuppressWarnings\s*\(\s*["']([^"']*)["']\s*\)/i; + /** * Generate an array of fixes, if possible. * @returns @@ -106,6 +112,7 @@ export class _PmdFixGenerator extends FixGenerator { fixes.push(this.generateLineLevelSuppression()); processedLines.add(lineNumber); } + fixes.push(this.generateClassLevelSuppression()); } return fixes; } @@ -139,6 +146,159 @@ export class _PmdFixGenerator extends FixGenerator { title: 'Clear Single Diagnostic', arguments: [this.document.uri, this.diagnostic.range] }; + return action; } + + public generateClassLevelSuppression(): vscode.CodeAction { + // Find the end-of-line position of the class declaration where the diagnostic is found. + const classStartPosition = this.findClassStartPosition(this.diagnostic, this.document); + + const action = new vscode.CodeAction(messages.fixer.supressOnClass, vscode.CodeActionKind.QuickFix); + action.edit = new vscode.WorkspaceEdit(); + + // Determine the appropriate suppression rule based on the type of diagnostic.code + let suppressionRule: string; + if (typeof this.diagnostic.code == 'object' && 'value' in this.diagnostic.code) { + suppressionRule = `PMD.${this.diagnostic.code.value}`; + } else { + suppressionRule = `PMD`; + } + + // Extract text from the start to end of the class declaration to search for existing suppressions + const classText = this.findLineBeforeClassStartDeclaration(classStartPosition, this.document); + const suppressionMatch = classText.match(this.suppressionRegex); + + if (suppressionMatch) { + // If @SuppressWarnings exists, check if the rule is already present + const existingRules = suppressionMatch[1].split(',').map(rule => rule.trim()); + if (!existingRules.includes(suppressionRule)) { + // If the rule is not present, add it to the existing @SuppressWarnings + const updatedRules = [...existingRules, suppressionRule].join(', '); + const updatedSuppression = this.generateUpdatedSuppressionTag(updatedRules, this.document.languageId); + const suppressionStartPosition = this.document.positionAt(classText.indexOf(suppressionMatch[0])); + const suppressionEndPosition = this.document.positionAt(classText.indexOf(suppressionMatch[0]) + suppressionMatch[0].length); + const suppressionRange = new vscode.Range(suppressionStartPosition, suppressionEndPosition); + action.edit.replace(this.document.uri, suppressionRange, updatedSuppression); + } + } else { + // If @SuppressWarnings does not exist, insert a new one + const newSuppression = this.generateNewSuppressionTag(suppressionRule, this.document.languageId); + action.edit.insert(this.document.uri, classStartPosition, newSuppression); + } + + action.diagnostics = [this.diagnostic]; + action.command = { + command: Constants.COMMAND_REMOVE_DIAGNOSTICS_ON_SELECTED_FILE, + title: 'Remove diagnostics for this file', + arguments: [this.document.uri] + }; + + return action; + } + + public generateUpdatedSuppressionTag(updatedRules: string, lang: string) { + if (lang === 'apex') { + return `@SuppressWarnings('${updatedRules}')`; + } else if (lang === 'java') { + return `@SuppressWarnings("${updatedRules}")`; + } + return ''; + } + + public generateNewSuppressionTag(suppressionRule: string, lang: string) { + if (lang === 'apex') { + return `@SuppressWarnings('${suppressionRule}')\n`; + } else if (lang === 'java') { + return `@SuppressWarnings("${suppressionRule}")\n`; + } + return ''; + } + + /** + * Finds the start position of the class in the document. + * Assumes that the class declaration starts with the keyword "class". + * @returns The position at the start of the class. + */ + public findClassStartPosition(diagnostic: vscode.Diagnostic, document: vscode.TextDocument): vscode.Position { + const text = document.getText(); + const diagnosticLine = diagnostic.range.start.line; + + // Split the text into lines for easier processing + const lines = text.split('\n'); + let classStartLine: number | undefined; + + let inBlockComment = false; + + // Iterate from the diagnostic line upwards to find the class declaration + for (let lineNumber = 0; lineNumber <= diagnosticLine; lineNumber++) { + const line = lines[lineNumber]; + + // Check if this line is the start of a block comment + if (!inBlockComment && line.match(this.blockCommentStartPattern)) { + inBlockComment = true; + continue; + } + + // Check if we are in the end of block comment + if (inBlockComment && line.match(this.blockCommentEndPattern)) { + inBlockComment = false; + continue; + } + + // Skip single-line comments + if (line.match(this.singleLineCommentPattern)) { + continue; + } + + // Skip block comment in a single line + if (line.match(this.blockCommentEndPattern) && line.match(this.blockCommentStartPattern)) { + continue; + } + + const match = line.match(this.classDeclarationPattern); + if (!inBlockComment && match && !this.isWithinQuotes(line, match.index)) { + classStartLine = lineNumber; + break; + } + } + + if (classStartLine !== undefined) { + return new vscode.Position(classStartLine, 0); + } + + // Default to the start of the document if class is not found + return new vscode.Position(0, 0); + } + + /** + * Finds the entire line that is one line above a class declaration statement. + * @returns The text of the line that is one line above the class declaration. + */ + public findLineBeforeClassStartDeclaration(classStartPosition: vscode.Position, document: vscode.TextDocument): string { + // Ensure that there is a line before the class declaration + if (classStartPosition.line > 0) { + const lineBeforeClassPosition = classStartPosition.line - 1; + const lineBeforeClass = document.lineAt(lineBeforeClassPosition); + return lineBeforeClass.text; + } + + // Return an empty string if it's the first line of the document + return ''; + } + + /** + * Helper function to check if match is within quotes + * @param line + * @param matchIndex + * @returns + */ + public isWithinQuotes(line: string, matchIndex: number): boolean { + const beforeMatch = line.slice(0, matchIndex); + const singleQuotesBefore = (beforeMatch.match(/'/g) || []).length; + const doubleQuotesBefore = (beforeMatch.match(/"/g) || []).length; + + // Check if the number of quotes before the match is odd (inside quotes) + return singleQuotesBefore % 2 !== 0 || doubleQuotesBefore % 2 !== 0 + } } diff --git a/src/lib/messages.ts b/src/lib/messages.ts index fc32ddf..2df5ea2 100644 --- a/src/lib/messages.ts +++ b/src/lib/messages.ts @@ -32,7 +32,8 @@ export const messages = { existingDfaRunText: "A Salesforce Graph Engine analysis is already running. Cancel it by clicking in the Status Bar.", }, fixer: { - supressOnLine: "Suppress violations on this line." + supressOnLine: "Suppress violations on this line.", + supressOnClass: "***Suppress violations on this class.***" }, diagnostics: { messageGenerator: (severity: number, message: string) => `Sev${severity}: ${message}`, diff --git a/src/test/suite/fixer.test.ts b/src/test/suite/fixer.test.ts index c5448a6..0e22e42 100644 --- a/src/test/suite/fixer.test.ts +++ b/src/test/suite/fixer.test.ts @@ -7,6 +7,7 @@ import * as vscode from 'vscode'; import {expect} from 'chai'; import path = require('path'); +import Sinon = require('sinon'); import {messages} from '../../lib/messages'; import {_NoOpFixGenerator, _PmdFixGenerator} from '../../lib/fixer'; @@ -99,7 +100,7 @@ suite('fixer.ts', () => { const fixes: vscode.CodeAction[] = fixGenerator.generateFixes(existingFixes); // We expect to get one fix, to inject the suppression at the end of the line. - expect(fixes).to.have.lengthOf(1, 'Wrong action count'); + expect(fixes).to.have.lengthOf(2, 'Wrong action count'); const fix = fixes[0].edit.get(fileUri)[0]; expect(fix.newText).to.equal(' // NOPMD', 'Wrong suppression added'); expect(fix.range.start.isEqual(new vscode.Position(7, Number.MAX_SAFE_INTEGER))).to.equal(true, 'Should be at the end of the violation line'); @@ -123,8 +124,8 @@ suite('fixer.ts', () => { // Attempt to generate fixes for the file. const fixes: vscode.CodeAction[] = fixGenerator.generateFixes(existingFixes); - // We expect to get no fix, since there is already a fix - expect(fixes).to.have.lengthOf(0, 'Wrong action count'); + // We expect to get one fix (class level suppression), no new line level suppression added since there is already a fix + expect(fixes).to.have.lengthOf(1, 'Wrong action count'); }); /* @@ -154,7 +155,338 @@ suite('fixer.ts', () => { }); */ }); + + suite('Class-level suppression', () => { + suite('#findClassStartPosition()', () => { + test('Should find class start position above the diagnostic line', async () => { + const fileUri = vscode.Uri.file(path.join(codeFixturesPath, 'MyClass1.cls')); + + let doc: vscode.TextDocument; + doc = await vscode.workspace.openTextDocument(fileUri); + const diag = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(7, 4), + new vscode.Position(7, 10) + ), + 'This message is unimportant', + vscode.DiagnosticSeverity.Warning + ); + + // Instantiate our fixer + const fixGenerator: _PmdFixGenerator = new _PmdFixGenerator(doc, diag); + + // Call findClassStartPosition method + const position = fixGenerator.findClassStartPosition(diag, doc); + + // Verify the position is correct + expect(position.line).to.equal(6); + expect(position.character).to.equal(0); + }); + + test('Should ignore class defined in single line comment', async () => { + const fileUri = vscode.Uri.file(path.join(codeFixturesPath, 'MyClass2.cls')); + + let doc: vscode.TextDocument; + doc = await vscode.workspace.openTextDocument(fileUri); + const diag = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(10, 0), + new vscode.Position(10, 1) + ), + 'This message is unimportant', + vscode.DiagnosticSeverity.Warning + ); + + // Instantiate our fixer + const fixGenerator: _PmdFixGenerator = new _PmdFixGenerator(doc, diag); + + // Call findClassStartPosition method + const position = fixGenerator.findClassStartPosition(diag, doc); + + // Verify the position is correct + expect(position.line).to.equal(6); + expect(position.character).to.equal(0); + }); + + test('Should ignore class defined in a block comment comment', async () => { + const fileUri = vscode.Uri.file(path.join(codeFixturesPath, 'MyClass2.cls')); + + let doc: vscode.TextDocument; + doc = await vscode.workspace.openTextDocument(fileUri); + const diag = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(17, 0), + new vscode.Position(17, 1) + ), + 'This message is unimportant', + vscode.DiagnosticSeverity.Warning + ); + + // Instantiate our fixer + const fixGenerator: _PmdFixGenerator = new _PmdFixGenerator(doc, diag); + + // Call findClassStartPosition method + const position = fixGenerator.findClassStartPosition(diag, doc); + + // Verify the position is correct + expect(position.line).to.equal(6); + expect(position.character).to.equal(0); + }); + + test('Should ignore class defined as a string', async () => { + const fileUri = vscode.Uri.file(path.join(codeFixturesPath, 'MyClass2.cls')); + + let doc: vscode.TextDocument; + doc = await vscode.workspace.openTextDocument(fileUri); + const diag = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(23, 0), + new vscode.Position(23, 1) + ), + 'This message is unimportant', + vscode.DiagnosticSeverity.Warning + ); + + // Instantiate our fixer + const fixGenerator: _PmdFixGenerator = new _PmdFixGenerator(doc, diag); + + // Call findClassStartPosition method + const position = fixGenerator.findClassStartPosition(diag, doc); + + // Verify the position is correct + expect(position.line).to.equal(6); + expect(position.character).to.equal(0); + }); + test('Should ignore inner class', async () => { + const fileUri = vscode.Uri.file(path.join(codeFixturesPath, 'MyClass2.cls')); + + let doc: vscode.TextDocument; + doc = await vscode.workspace.openTextDocument(fileUri); + const diag = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(27, 0), + new vscode.Position(27, 1) + ), + 'This message is unimportant', + vscode.DiagnosticSeverity.Warning + ); + + // Instantiate our fixer + const fixGenerator: _PmdFixGenerator = new _PmdFixGenerator(doc, diag); + + // Call findClassStartPosition method + const position = fixGenerator.findClassStartPosition(diag, doc); + + // Verify the position is correct + expect(position.line).to.equal(6); + expect(position.character).to.equal(0); + }); + }); + }); + suite('#generateNewSuppressionTag()', () => { + // Instantiate our fixer + const fixGenerator: _PmdFixGenerator = new _PmdFixGenerator(null, null); + + test('Should generate the correct suppression tag for Apex language', async () => { + const suppressionRule = 'rule1'; + const lang = 'apex'; + const expectedSuppressionTag = `@SuppressWarnings('${suppressionRule}')\n`; + expect(fixGenerator.generateNewSuppressionTag(suppressionRule, lang)).to.equal(expectedSuppressionTag); + }); + + test('Should generate the correct suppression tag for Java language', async () => { + const suppressionRule = 'rule2'; + const lang = 'java'; + const expectedSuppressionTag = `@SuppressWarnings("${suppressionRule}")\n`; + expect(fixGenerator.generateNewSuppressionTag(suppressionRule, lang)).to.equal(expectedSuppressionTag); + }); + + test('Should return an empty string for unsupported languages', async () => { + const suppressionRule = 'rule3'; + const lang = 'python'; // Assuming python as an unsupported language + expect(fixGenerator.generateNewSuppressionTag(suppressionRule, lang)).to.equal(''); + }); + }); + suite('#generateUpdatedSuppressionTag()', () => { + // Instantiate our fixer + const fixGenerator: _PmdFixGenerator = new _PmdFixGenerator(null, null); + + test('Should generate the correct suppression tag for Apex language with single quotes', async () => { + const updatedRules = 'rule1'; + const lang = 'apex'; + const expectedSuppressionTag = `@SuppressWarnings('${updatedRules}')`; + expect(fixGenerator.generateUpdatedSuppressionTag(updatedRules, lang)).to.equal(expectedSuppressionTag); + }); + + test('Should generate the correct suppression tag for Java language with double quotes', async () => { + const updatedRules = 'rule2'; + const lang = 'java'; + const expectedSuppressionTag = `@SuppressWarnings("${updatedRules}")`; + expect(fixGenerator.generateUpdatedSuppressionTag(updatedRules, lang)).to.equal(expectedSuppressionTag); + }); + + test('Should return an empty string for unsupported languages', async () => { + const updatedRules = 'rule3'; + const lang = 'python'; // Assuming python as an unsupported language + expect(fixGenerator.generateUpdatedSuppressionTag(updatedRules, lang)).to.equal(''); + }); + }); + suite('#findLineBeforeClassStartDeclaration()', () => { + // Instantiate our fixer + const fixGenerator: _PmdFixGenerator = new _PmdFixGenerator(null, null); + + test('Should find the correct line before class start declaration when it is not the first line', async () => { + const classStartPosition = new vscode.Position(2, 0); + const document = { + lineAt: (lineNumber: number) => { + return { + // Simulating document content and for easy testing, line number starts at 0 + text: `This is line ${lineNumber}`, + }; + }, + } as vscode.TextDocument; + + // Call findLineBeforeClassStartDeclaration method + const line = fixGenerator.findLineBeforeClassStartDeclaration(classStartPosition, document); + + // Verify the line content is correct + expect(line).to.equal('This is line 1'); + }); + + test('Should return empty string when class declaration is the first line', async () => { + const classStartPosition = new vscode.Position(0, 0); + const document = { + lineAt: (lineNumber: number) => { + return { + // Simulating document content and for easy testing, line number starts at 0 + text: `This is line ${lineNumber}`, + }; + }, + } as vscode.TextDocument; + + // Call findLineBeforeClassStartDeclaration method + const line = fixGenerator.findLineBeforeClassStartDeclaration(classStartPosition, document); + + // Verify the line content is correct + expect(line).to.equal(''); + }); + }); + suite('#isWithinQuotes()', () => { + // Instantiate our fixer + const fixGenerator: _PmdFixGenerator = new _PmdFixGenerator(null, null); + + test('Should return true if the match is within single quotes', async () => { + const line = "This is 'a matching string'"; + const matchIndex = 15; // Index where match occurs within the string + const isWithin = fixGenerator.isWithinQuotes(line, matchIndex); + expect(isWithin).to.equal(true); + }); + + test('Should return true if the match is within double quotes', async () => { + const line = 'This is "a matching string"'; + const matchIndex = 21; // Index where match occurs within the string + const isWithin = fixGenerator.isWithinQuotes(line, matchIndex); + expect(isWithin).to.equal(true); + }); + + test('Should return false if the match is not within quotes', async () => { + const line = 'This is a line without quotes'; + const matchIndex = 5; // Index where match occurs within the string + const isWithin = fixGenerator.isWithinQuotes(line, matchIndex); + expect(isWithin).to.equal(false); + }); + + test('Should return false if the match is at the start of a string within quotes', async () => { + const line = "'quotes' is at the start of a string"; + const matchIndex = 10; // Index where match occurs within the string and it is after the quotes + const isWithin = fixGenerator.isWithinQuotes(line, matchIndex); + expect(isWithin).to.equal(false); + }); + + test('Should return true if the match is at the start of a string but quotes is not closed', async () => { + // This is an extreme case where someone opens a quote and has class defined in it + // and the closure of the quote is not on the same line. + const line = "'quotes is at the start of a string"; + const matchIndex = 10; // Index where match occurs within the string and it is after the quotes + const isWithin = fixGenerator.isWithinQuotes(line, matchIndex); + expect(isWithin).to.equal(true); + }); + + }); + }); }); + suite('Regex Pattern Tests', () => { + let fixGenerator: _PmdFixGenerator; + + setup(() => { + fixGenerator = new _PmdFixGenerator(null, null); + }); + + test('singleLineCommentPattern matches single-line comments', () => { + const pattern = fixGenerator.singleLineCommentPattern; + + // Matching cases + expect(pattern.test('// This is a single-line comment')).to.be.true; + + // Non-matching cases + expect(pattern.test('This is not a comment')).to.be.false; + expect(pattern.test('/* This is a block comment start */')).to.be.false; + }); + + test('blockCommentStartPattern matches block comment starts', () => { + const pattern = fixGenerator.blockCommentStartPattern; + + // Matching cases + expect(pattern.test('/* This is a block comment start')).to.be.true; + expect(pattern.test(' /* This is an indented block comment start')).to.be.true; + + // Non-matching cases + expect(pattern.test('This is not a comment')).to.be.false; + expect(pattern.test('// This is a single-line comment')).to.be.false; + expect(pattern.test('*/ This is a block comment end')).to.be.false; + }); + + test('blockCommentEndPattern matches block comment ends', () => { + const pattern = fixGenerator.blockCommentEndPattern; + + // Matching cases + expect(pattern.test('*/')).to.be.true; + expect(pattern.test(' */ This is an indented block comment end')).to.be.true; + + // Non-matching cases + expect(pattern.test('This is not a comment')).to.be.false; + expect(pattern.test('// This is a single-line comment')).to.be.false; + expect(pattern.test('/* This is a block comment start')).to.be.false; + }); + + test('classDeclarationPattern matches class declarations', () => { + const pattern = fixGenerator.classDeclarationPattern; + + // Matching cases + expect(pattern.test('public class MyClass')).to.be.true; + expect(pattern.test('final public class MyClass')).to.be.true; + expect(pattern.test(' private static class MyClass')).to.be.true; + + // Non-matching cases + expect(pattern.test('class="MyClass"')).to.be.false; // HTML-like attribute + expect(pattern.test('String myClass = "some value"')).to.be.false; + }); + + test('suppressionRegex matches @SuppressWarnings annotations', () => { + const pattern = fixGenerator.suppressionRegex; + + // Matching cases + expect(pattern.test("@SuppressWarnings('PMD.Rule')")).to.be.true; + expect(pattern.test("@suppresswarnings('pmd.rule')")).to.be.true; + expect(pattern.test("@suppresswarnings('PMD.Rule')")).to.be.true; + expect(pattern.test('@SuppressWarnings("PMD.Rule")')).to.be.true; + + // Non-matching cases + expect(pattern.test('This is not a suppression annotation')).to.be.false; + expect(pattern.test('@SuppressWarnings')).to.be.false; + expect(pattern.test('SuppressWarnings("PMD.Rule")')).to.be.false; // Missing '@' + }); + }); }); });