diff --git a/README.md b/README.md index a865b7c..96aa2ab 100644 --- a/README.md +++ b/README.md @@ -40,11 +40,31 @@ The `forcedotcom/run-code-analyzer@v2` GitHub Action is based on [Salesforce Cod * The number of Low (4) severity violations found. * `num-sev5-violations` * The number of Info (5) severity violations found. +* `num-violations-in-changed-files` + * The total number of violations found in files changed by the pull request. + * Only available when running on a pull request with a `github-token` provided. +* `num-sev1-violations-in-changed-files` + * The number of Critical (1) severity violations found in files changed by the pull request. + * Only available when running on a pull request with a `github-token` provided. +* `num-sev2-violations-in-changed-files` + * The number of High (2) severity violations found in files changed by the pull request. + * Only available when running on a pull request with a `github-token` provided. +* `num-sev3-violations-in-changed-files` + * The number of Medium (3) severity violations found in files changed by the pull request. + * Only available when running on a pull request with a `github-token` provided. +* `num-sev4-violations-in-changed-files` + * The number of Low (4) severity violations found in files changed by the pull request. + * Only available when running on a pull request with a `github-token` provided. +* `num-sev5-violations-in-changed-files` + * The number of Info (5) severity violations found in files changed by the pull request. + * Only available when running on a pull request with a `github-token` provided. * `review-id` * If the action created a pull request review, this is its ID. This `run-code-analyzer@v2` action doesn't exit your GitHub workflow when it finds violations. We recommend that you add a subsequent step to your workflow that uses the available outputs to determine how your workflow should proceed. +**Tip:** The `*-in-changed-files` outputs are useful when introducing code analysis to a legacy codebase with existing violations. You can ensure new code meets quality standards without blocking PRs due to pre-existing issues. See "Option 2" in the example below. + ## Environment Prerequisites The [Salesforce Code Analyzer v5.x](https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/code-analyzer.html) and its bundled engines can each have their own set of requirements in order to run successfully. We recommend that you set up your GitHub runner(s) with this software: * `node` version 20.9.0 or greater @@ -99,7 +119,8 @@ The [Salesforce Code Analyzer v5.x](https://developer.salesforce.com/docs/platfo results-artifact-name: salesforce-code-analyzer-results github-token: ${{ github.token }} - - name: Check the Outputs to Determine Whether to Fail + # Option 1: Quality gate on ALL files in the repository + - name: Fail on Critical/High Violations (All Files) if: | steps.run-code-analyzer.outputs.exit-code > 0 || steps.run-code-analyzer.outputs.num-sev1-violations > 0 || @@ -107,6 +128,13 @@ The [Salesforce Code Analyzer v5.x](https://developer.salesforce.com/docs/platfo steps.run-code-analyzer.outputs.num-violations > 10 run: exit 1 + # Option 2: Quality gate on CHANGED files only (useful for legacy codebases) + - name: Fail on Critical/High Violations (Changed Files Only) + if: | + steps.run-code-analyzer.outputs.num-sev1-violations-in-changed-files > 0 || + steps.run-code-analyzer.outputs.num-sev2-violations-in-changed-files > 0 + run: exit 1 + # Version: v1 The `forcedotcom/run-code-analyzer@v1` GitHub Action is based on [Salesforce Code Analyzer v4.x](https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/code-analyzer-3x.html), which is the original `@salesforce/sfdx-scanner` Salesforce CLI plugin. diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index 62672d8..0cc396d 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -452,4 +452,142 @@ describe('main run Tests', () => { expect(dependencies.failCallHistory).toHaveLength(1) expect(dependencies.failCallHistory[0].failMessage).toContain(MESSAGE_FCNS.FILE_NOT_FOUND('userResults.xml')) }) + + it('When running on a pull request with token, changed files outputs are set correctly', async () => { + dependencies.getInputsReturnValue = { + runArguments: '--view detail --output-file sfca_results.json', + resultsArtifactName: 'salesforce-code-analyzer-results', + githubToken: 'dummyToken' + } + dependencies.isPullRequestReturnValue = true + // 'fakeFile' matches the default file in FakeViolationLocation, so all 15 violations are in changed files + dependencies.getChangedFilesCallback = async () => ['fakeFile'] + dependencies.createPullRequestReviewCallback = async () => 7 + await main.run(dependencies, commandExecutor, resultsFactory, summarizer) + + // Verify the new changed files outputs are set + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-violations-in-changed-files', + value: '15' // All 15 violations are in 'fakeFile' + }) + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-sev1-violations-in-changed-files', + value: '1' + }) + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-sev2-violations-in-changed-files', + value: '2' + }) + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-sev3-violations-in-changed-files', + value: '3' + }) + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-sev4-violations-in-changed-files', + value: '4' + }) + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-sev5-violations-in-changed-files', + value: '5' + }) + + // Also verify the regular outputs are still set + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-violations', + value: '15' + }) + }) + + it('When running on a pull request with no matching changed files, changed files outputs show zero', async () => { + dependencies.getInputsReturnValue = { + runArguments: '--view detail --output-file sfca_results.json', + resultsArtifactName: 'salesforce-code-analyzer-results', + githubToken: 'dummyToken' + } + dependencies.isPullRequestReturnValue = true + // 'otherFile.ts' does NOT match 'fakeFile', so no violations are in changed files + dependencies.getChangedFilesCallback = async () => ['otherFile.ts'] + dependencies.createPullRequestReviewCallback = async () => 7 + await main.run(dependencies, commandExecutor, resultsFactory, summarizer) + + // Verify the changed files outputs show zero + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-violations-in-changed-files', + value: '0' + }) + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-sev1-violations-in-changed-files', + value: '0' + }) + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-sev2-violations-in-changed-files', + value: '0' + }) + + // Regular outputs should still show all violations + expect(dependencies.setOutputCallHistory).toContainEqual({ + name: 'num-violations', + value: '15' + }) + }) + + it('When NOT running on a pull request, changed files outputs are NOT set', async () => { + dependencies.getInputsReturnValue = { + runArguments: '--view detail --output-file sfca_results.json', + resultsArtifactName: 'salesforce-code-analyzer-results', + githubToken: 'dummyToken' + } + dependencies.isPullRequestReturnValue = false + await main.run(dependencies, commandExecutor, resultsFactory, summarizer) + + // Changed files outputs should NOT be in the output history + const outputNames = dependencies.setOutputCallHistory.map(o => o.name) + expect(outputNames).not.toContain('num-violations-in-changed-files') + expect(outputNames).not.toContain('num-sev1-violations-in-changed-files') + expect(outputNames).not.toContain('num-sev2-violations-in-changed-files') + + // Regular outputs should still be set + expect(outputNames).toContain('num-violations') + expect(outputNames).toContain('num-sev1-violations') + }) + + it('When running on a pull request without github token, changed files outputs are NOT set', async () => { + dependencies.getInputsReturnValue = { + runArguments: '--view detail --output-file sfca_results.json', + resultsArtifactName: 'salesforce-code-analyzer-results' + // No githubToken + } + dependencies.isPullRequestReturnValue = true + await main.run(dependencies, commandExecutor, resultsFactory, summarizer) + + // Changed files outputs should NOT be in the output history + const outputNames = dependencies.setOutputCallHistory.map(o => o.name) + expect(outputNames).not.toContain('num-violations-in-changed-files') + expect(outputNames).not.toContain('num-sev1-violations-in-changed-files') + + // Regular outputs should still be set + expect(outputNames).toContain('num-violations') + }) + + it('When getChangedFiles fails, changed files outputs are NOT set', async () => { + dependencies.getInputsReturnValue = { + runArguments: '--view detail --output-file sfca_results.json', + resultsArtifactName: 'salesforce-code-analyzer-results', + githubToken: 'dummyToken' + } + dependencies.isPullRequestReturnValue = true + dependencies.getChangedFilesCallback = async () => { + throw new Error('Failed to get changed files') + } + await main.run(dependencies, commandExecutor, resultsFactory, summarizer) + + // Changed files outputs should NOT be in the output history + const outputNames = dependencies.setOutputCallHistory.map(o => o.name) + expect(outputNames).not.toContain('num-violations-in-changed-files') + expect(outputNames).not.toContain('num-sev1-violations-in-changed-files') + + // Warning should be logged + expect(dependencies.warnCallHistory).toHaveLength(1) + expect(dependencies.warnCallHistory[0].warnMessage).toContain('Failed to get changed files') + }) }) diff --git a/action.yml b/action.yml index ac85482..92189e2 100644 --- a/action.yml +++ b/action.yml @@ -38,17 +38,29 @@ outputs: exit-code: description: The Salesforce Code Analyzer execution exit code. num-violations: - description: The total number of violations found. + description: The total number of violations found across all files. num-sev1-violations: - description: The number of Critical (1) severity violations found. + description: The number of Critical (1) severity violations found across all files. num-sev2-violations: - description: The number of High (2) severity violations found. + description: The number of High (2) severity violations found across all files. num-sev3-violations: - description: The number of Medium (3) severity violations found. + description: The number of Medium (3) severity violations found across all files. num-sev4-violations: - description: The number of Low (4) severity violations found. + description: The number of Low (4) severity violations found across all files. num-sev5-violations: - description: The number of Info (5) severity violations found. + description: The number of Info (5) severity violations found across all files. + num-violations-in-changed-files: + description: The total number of violations found in files changed by the pull request. Only available when running on a pull request with a github-token provided. + num-sev1-violations-in-changed-files: + description: The number of Critical (1) severity violations found in files changed by the pull request. Only available when running on a pull request with a github-token provided. + num-sev2-violations-in-changed-files: + description: The number of High (2) severity violations found in files changed by the pull request. Only available when running on a pull request with a github-token provided. + num-sev3-violations-in-changed-files: + description: The number of Medium (3) severity violations found in files changed by the pull request. Only available when running on a pull request with a github-token provided. + num-sev4-violations-in-changed-files: + description: The number of Low (4) severity violations found in files changed by the pull request. Only available when running on a pull request with a github-token provided. + num-sev5-violations-in-changed-files: + description: The number of Info (5) severity violations found in files changed by the pull request. Only available when running on a pull request with a github-token provided. review-id: description: If this action created a pull request review, this is its ID. diff --git a/dist/index.js b/dist/index.js index 1561a32..f65a163 100644 --- a/dist/index.js +++ b/dist/index.js @@ -102801,15 +102801,19 @@ async function run(dependencies, commandExecutor, resultsFactory, summarizer) { } const summaryMarkdown = summarizer.createSummaryMarkdown(results, changedFiles); if (couldReadChangedFiles) { - const summaryLink = await dependencies.createActionSummaryLink(inputs.githubToken); + // Calculate violations in changed files const changedFilesSet = new Set(changedFiles); - const violationsInChangedFilesCount = results + const violationsInChangedFiles = results .getViolationsSortedBySeverity() .filter((v) => v .getLocations() .map(l => l.getFile()) - .some(f => f && changedFilesSet.has(f))).length; - const summaryBody = constants_1.MESSAGE_FCNS.REVIEW_BODY(results.getTotalViolationCount(), violationsInChangedFilesCount, summaryLink); + .some(f => f && changedFilesSet.has(f))); + const severityCounts = countViolationsBySeverity(violationsInChangedFiles); + setChangedFilesOutputs(dependencies, severityCounts); + // Create PR review + const summaryLink = await dependencies.createActionSummaryLink(inputs.githubToken); + const summaryBody = constants_1.MESSAGE_FCNS.REVIEW_BODY(results.getTotalViolationCount(), severityCounts.total, summaryLink); try { dependencies.info(constants_1.MESSAGES.ATTEMPTING_TO_CREATE_PR_REVIEW); const reviewId = await dependencies.createPullRequestReview(inputs.githubToken, summaryBody); @@ -102860,6 +102864,32 @@ function assertFileExists(dependencies, file) { throw new Error(constants_1.MESSAGE_FCNS.FILE_NOT_FOUND(file)); } } +function countViolationsBySeverity(violations) { + const countBySeverity = (sev) => violations.filter(v => v.getSeverity() === sev).length; + return { + sev1: countBySeverity(1), + sev2: countBySeverity(2), + sev3: countBySeverity(3), + sev4: countBySeverity(4), + sev5: countBySeverity(5), + total: violations.length + }; +} +function setChangedFilesOutputs(dependencies, counts) { + dependencies.setOutput('num-violations-in-changed-files', counts.total.toString()); + dependencies.setOutput('num-sev1-violations-in-changed-files', counts.sev1.toString()); + dependencies.setOutput('num-sev2-violations-in-changed-files', counts.sev2.toString()); + dependencies.setOutput('num-sev3-violations-in-changed-files', counts.sev3.toString()); + dependencies.setOutput('num-sev4-violations-in-changed-files', counts.sev4.toString()); + dependencies.setOutput('num-sev5-violations-in-changed-files', counts.sev5.toString()); + dependencies.info(`changed files outputs:\n` + + ` num-violations-in-changed-files: ${counts.total}\n` + + ` num-sev1-violations-in-changed-files: ${counts.sev1}\n` + + ` num-sev2-violations-in-changed-files: ${counts.sev2}\n` + + ` num-sev3-violations-in-changed-files: ${counts.sev3}\n` + + ` num-sev4-violations-in-changed-files: ${counts.sev4}\n` + + ` num-sev5-violations-in-changed-files: ${counts.sev5}`); +} /***/ }), diff --git a/src/main.ts b/src/main.ts index 2a60ab0..b3a4a58 100644 --- a/src/main.ts +++ b/src/main.ts @@ -95,19 +95,25 @@ export async function run( const summaryMarkdown = summarizer.createSummaryMarkdown(results, changedFiles) if (couldReadChangedFiles) { - const summaryLink: string = await dependencies.createActionSummaryLink(inputs.githubToken) + // Calculate violations in changed files const changedFilesSet: Set = new Set(changedFiles) - const violationsInChangedFilesCount: number = results + const violationsInChangedFiles: Violation[] = results .getViolationsSortedBySeverity() .filter((v: Violation): boolean => v .getLocations() .map(l => l.getFile()) .some(f => f && changedFilesSet.has(f)) - ).length + ) + + const severityCounts = countViolationsBySeverity(violationsInChangedFiles) + setChangedFilesOutputs(dependencies, severityCounts) + + // Create PR review + const summaryLink: string = await dependencies.createActionSummaryLink(inputs.githubToken) const summaryBody = MESSAGE_FCNS.REVIEW_BODY( results.getTotalViolationCount(), - violationsInChangedFilesCount, + severityCounts.total, summaryLink ) try { @@ -166,3 +172,42 @@ function assertFileExists(dependencies: Dependencies, file: string): void { throw new Error(MESSAGE_FCNS.FILE_NOT_FOUND(file)) } } + +interface SeverityCounts { + sev1: number + sev2: number + sev3: number + sev4: number + sev5: number + total: number +} + +function countViolationsBySeverity(violations: Violation[]): SeverityCounts { + const countBySeverity = (sev: number): number => violations.filter(v => v.getSeverity() === sev).length + return { + sev1: countBySeverity(1), + sev2: countBySeverity(2), + sev3: countBySeverity(3), + sev4: countBySeverity(4), + sev5: countBySeverity(5), + total: violations.length + } +} + +function setChangedFilesOutputs(dependencies: Dependencies, counts: SeverityCounts): void { + dependencies.setOutput('num-violations-in-changed-files', counts.total.toString()) + dependencies.setOutput('num-sev1-violations-in-changed-files', counts.sev1.toString()) + dependencies.setOutput('num-sev2-violations-in-changed-files', counts.sev2.toString()) + dependencies.setOutput('num-sev3-violations-in-changed-files', counts.sev3.toString()) + dependencies.setOutput('num-sev4-violations-in-changed-files', counts.sev4.toString()) + dependencies.setOutput('num-sev5-violations-in-changed-files', counts.sev5.toString()) + dependencies.info( + `changed files outputs:\n` + + ` num-violations-in-changed-files: ${counts.total}\n` + + ` num-sev1-violations-in-changed-files: ${counts.sev1}\n` + + ` num-sev2-violations-in-changed-files: ${counts.sev2}\n` + + ` num-sev3-violations-in-changed-files: ${counts.sev3}\n` + + ` num-sev4-violations-in-changed-files: ${counts.sev4}\n` + + ` num-sev5-violations-in-changed-files: ${counts.sev5}` + ) +}