| b69ab31 | | | 1 | /** |
| b69ab31 | | | 2 | * Copyright (c) Meta Platforms, Inc. and affiliates. |
| b69ab31 | | | 3 | * |
| b69ab31 | | | 4 | * This source code is licensed under the MIT license found in the |
| b69ab31 | | | 5 | * LICENSE file in the root directory of this source tree. |
| b69ab31 | | | 6 | */ |
| b69ab31 | | | 7 | |
| b69ab31 | | | 8 | import type {Tracker} from 'isl-server/src/analytics/tracker'; |
| b69ab31 | | | 9 | import type {UseUncommittedSelection} from './partialSelection'; |
| b69ab31 | | | 10 | import type {CommitInfo, Diagnostic, DiagnosticAllowlist} from './types'; |
| b69ab31 | | | 11 | |
| b69ab31 | | | 12 | import * as stylex from '@stylexjs/stylex'; |
| b69ab31 | | | 13 | import {Checkbox} from 'isl-components/Checkbox'; |
| b69ab31 | | | 14 | import {Column, Row} from 'isl-components/Flex'; |
| b69ab31 | | | 15 | import {Icon} from 'isl-components/Icon'; |
| b69ab31 | | | 16 | import {Subtle} from 'isl-components/Subtle'; |
| b69ab31 | | | 17 | import {Tooltip} from 'isl-components/Tooltip'; |
| b69ab31 | | | 18 | import {useAtom} from 'jotai'; |
| b69ab31 | | | 19 | import {basename} from 'shared/utils'; |
| b69ab31 | | | 20 | import {spacing} from '../../components/theme/tokens.stylex'; |
| b69ab31 | | | 21 | import serverAPI from './ClientToServerAPI'; |
| b69ab31 | | | 22 | import {Collapsable} from './Collapsable'; |
| b69ab31 | | | 23 | import {Internal} from './Internal'; |
| b69ab31 | | | 24 | import {tracker} from './analytics'; |
| b69ab31 | | | 25 | import {getFeatureFlag} from './featureFlags'; |
| b69ab31 | | | 26 | import {T, t} from './i18n'; |
| b69ab31 | | | 27 | import {localStorageBackedAtom, readAtom} from './jotaiUtils'; |
| b69ab31 | | | 28 | import platform from './platform'; |
| b69ab31 | | | 29 | import {uncommittedChangesWithPreviews} from './previews'; |
| b69ab31 | | | 30 | import {showModal} from './useModal'; |
| b69ab31 | | | 31 | |
| b69ab31 | | | 32 | export const shouldWarnAboutDiagnosticsAtom = localStorageBackedAtom<boolean>( |
| b69ab31 | | | 33 | 'isl.warn-about-diagnostics', |
| b69ab31 | | | 34 | true, |
| b69ab31 | | | 35 | ); |
| b69ab31 | | | 36 | |
| b69ab31 | | | 37 | const hideNonBlockingDiagnosticsAtom = localStorageBackedAtom<boolean>( |
| b69ab31 | | | 38 | 'isl.hide-non-blocking-diagnostics', |
| b69ab31 | | | 39 | true, |
| b69ab31 | | | 40 | ); |
| b69ab31 | | | 41 | |
| b69ab31 | | | 42 | const styles = stylex.create({ |
| b69ab31 | | | 43 | diagnosticList: { |
| b69ab31 | | | 44 | paddingInline: spacing.double, |
| b69ab31 | | | 45 | paddingBlock: spacing.half, |
| b69ab31 | | | 46 | gap: 0, |
| b69ab31 | | | 47 | }, |
| b69ab31 | | | 48 | nowrap: { |
| b69ab31 | | | 49 | whiteSpace: 'nowrap', |
| b69ab31 | | | 50 | }, |
| b69ab31 | | | 51 | diagnosticRow: { |
| b69ab31 | | | 52 | maxWidth: 'max(400px, 80vw)', |
| b69ab31 | | | 53 | padding: spacing.half, |
| b69ab31 | | | 54 | cursor: 'pointer', |
| b69ab31 | | | 55 | ':hover': { |
| b69ab31 | | | 56 | backgroundColor: 'var(--hover-darken)', |
| b69ab31 | | | 57 | }, |
| b69ab31 | | | 58 | }, |
| b69ab31 | | | 59 | allDiagnostics: { |
| b69ab31 | | | 60 | maxHeight: 'calc(100vh - 200px)', |
| b69ab31 | | | 61 | minHeight: '50px', |
| b69ab31 | | | 62 | overflowY: 'scroll', |
| b69ab31 | | | 63 | }, |
| b69ab31 | | | 64 | confirmCheckbox: { |
| b69ab31 | | | 65 | paddingTop: spacing.double, |
| b69ab31 | | | 66 | }, |
| b69ab31 | | | 67 | }); |
| b69ab31 | | | 68 | |
| b69ab31 | | | 69 | export function isBlockingDiagnostic( |
| b69ab31 | | | 70 | d: Diagnostic, |
| b69ab31 | | | 71 | /** Many diagnostics are low-quality and don't reflect what would appear on CI. |
| b69ab31 | | | 72 | * Start with an allowlist while we validate which signals are worthwhile. */ |
| b69ab31 | | | 73 | allowlistedCodesBySource: undefined | DiagnosticAllowlist = Internal.allowlistedDiagnosticCodes ?? |
| b69ab31 | | | 74 | undefined, |
| b69ab31 | | | 75 | ): boolean { |
| b69ab31 | | | 76 | if (allowlistedCodesBySource == null) { |
| b69ab31 | | | 77 | // In OSS, let's assume all errors are blocking. |
| b69ab31 | | | 78 | return true; |
| b69ab31 | | | 79 | } |
| b69ab31 | | | 80 | if (d.severity !== 'error' && d.severity !== 'warning') { |
| b69ab31 | | | 81 | return false; |
| b69ab31 | | | 82 | } |
| b69ab31 | | | 83 | if (allowlistedCodesBySource == null) { |
| b69ab31 | | | 84 | return true; |
| b69ab31 | | | 85 | } |
| b69ab31 | | | 86 | // source/code may be missing, but we still want to route that through the allowlist |
| b69ab31 | | | 87 | const source = d.source ?? 'undefined'; |
| b69ab31 | | | 88 | const code = d.code ?? 'undefined'; |
| b69ab31 | | | 89 | const relevantAllowlist = allowlistedCodesBySource.get(d.severity)?.get(source); |
| b69ab31 | | | 90 | return ( |
| b69ab31 | | | 91 | relevantAllowlist != null && |
| b69ab31 | | | 92 | (relevantAllowlist.allow |
| b69ab31 | | | 93 | ? relevantAllowlist.allow.has(code) === true |
| b69ab31 | | | 94 | : relevantAllowlist.block.has(code) === false) |
| b69ab31 | | | 95 | ); |
| b69ab31 | | | 96 | } |
| b69ab31 | | | 97 | |
| b69ab31 | | | 98 | function isErrorDiagnosticToLog(d: Diagnostic): boolean { |
| b69ab31 | | | 99 | return d.severity === 'error'; |
| b69ab31 | | | 100 | } |
| b69ab31 | | | 101 | |
| b69ab31 | | | 102 | /** Render diagnostic to a string, in the format `Source(Code): Snippet of error message` */ |
| b69ab31 | | | 103 | function previewDiagnostic(diagnostic: Diagnostic | undefined) { |
| b69ab31 | | | 104 | return diagnostic != null |
| b69ab31 | | | 105 | ? `${diagnostic.source}(${diagnostic.code}): ${diagnostic?.message.slice(0, 100)}` |
| b69ab31 | | | 106 | : undefined; |
| b69ab31 | | | 107 | } |
| b69ab31 | | | 108 | |
| b69ab31 | | | 109 | /** |
| b69ab31 | | | 110 | * Check IDE diagnostics for files that will be commit/amended/submitted, |
| b69ab31 | | | 111 | * to confirm if they intended the errors. |
| b69ab31 | | | 112 | */ |
| b69ab31 | | | 113 | export async function confirmNoBlockingDiagnostics( |
| b69ab31 | | | 114 | /** Check diagnostics for these selected files. */ |
| b69ab31 | | | 115 | selection: UseUncommittedSelection, |
| b69ab31 | | | 116 | /** If provided, warn for changes to files in this commit. Used when checking diagnostics when amending a commit. */ |
| b69ab31 | | | 117 | commit?: CommitInfo, |
| b69ab31 | | | 118 | ): Promise<boolean> { |
| b69ab31 | | | 119 | if (!readAtom(shouldWarnAboutDiagnosticsAtom)) { |
| b69ab31 | | | 120 | return true; |
| b69ab31 | | | 121 | } |
| b69ab31 | | | 122 | if (platform.platformName === 'vscode') { |
| b69ab31 | | | 123 | const allFiles = new Set<string>(); |
| b69ab31 | | | 124 | for (const file of readAtom(uncommittedChangesWithPreviews)) { |
| b69ab31 | | | 125 | if (selection.isFullyOrPartiallySelected(file.path)) { |
| b69ab31 | | | 126 | allFiles.add(file.path); |
| b69ab31 | | | 127 | } |
| b69ab31 | | | 128 | } |
| b69ab31 | | | 129 | for (const filePath of commit?.filePathsSample ?? []) { |
| b69ab31 | | | 130 | allFiles.add(filePath); |
| b69ab31 | | | 131 | } |
| b69ab31 | | | 132 | |
| b69ab31 | | | 133 | serverAPI.postMessage({ |
| b69ab31 | | | 134 | type: 'platform/checkForDiagnostics', |
| b69ab31 | | | 135 | paths: [...allFiles], |
| b69ab31 | | | 136 | }); |
| b69ab31 | | | 137 | const [result, enabled] = await Promise.all([ |
| b69ab31 | | | 138 | serverAPI.nextMessageMatching('platform/gotDiagnostics', () => true), |
| b69ab31 | | | 139 | getFeatureFlag( |
| b69ab31 | | | 140 | Internal.featureFlags?.ShowPresubmitDiagnosticsWarning, |
| b69ab31 | | | 141 | /* enable this feature in OSS */ true, |
| b69ab31 | | | 142 | ), |
| b69ab31 | | | 143 | ]); |
| b69ab31 | | | 144 | if (result.diagnostics.size > 0) { |
| b69ab31 | | | 145 | const allDiagnostics = [...result.diagnostics.values()]; |
| b69ab31 | | | 146 | const allBlockingErrors = allDiagnostics |
| b69ab31 | | | 147 | .map(value => value.filter(d => isBlockingDiagnostic(d))) |
| b69ab31 | | | 148 | .flat(); |
| b69ab31 | | | 149 | const totalErrors = allBlockingErrors.length; |
| b69ab31 | | | 150 | |
| b69ab31 | | | 151 | // It's useful to track even the diagnostics that are filtered out, to refine the allowlist in the future |
| b69ab31 | | | 152 | const unfilteredErrors = allDiagnostics |
| b69ab31 | | | 153 | .map(value => value.filter(isErrorDiagnosticToLog)) |
| b69ab31 | | | 154 | .flat(); |
| b69ab31 | | | 155 | |
| b69ab31 | | | 156 | const totalDiagnostics = allDiagnostics.flat().length; |
| b69ab31 | | | 157 | |
| b69ab31 | | | 158 | const firstError = allBlockingErrors[0]; |
| b69ab31 | | | 159 | const firstUnfilteredError = unfilteredErrors[0]; |
| b69ab31 | | | 160 | |
| b69ab31 | | | 161 | const childTracker = tracker.trackAsParent('DiagnosticsConfirmationOpportunity', { |
| b69ab31 | | | 162 | extras: { |
| b69ab31 | | | 163 | shown: enabled, |
| b69ab31 | | | 164 | unfilteredErrorCodes: [...new Set(unfilteredErrors.map(d => `${d.source}(${d.code})`))], |
| b69ab31 | | | 165 | filteredErrorCodes: [...new Set(allBlockingErrors.map(d => `${d.source}(${d.code})`))], |
| b69ab31 | | | 166 | sampleMessage: previewDiagnostic(firstError), |
| b69ab31 | | | 167 | unfilteredSampleMessage: previewDiagnostic(firstUnfilteredError), |
| b69ab31 | | | 168 | totalUnfilteredErrors: unfilteredErrors.length, |
| b69ab31 | | | 169 | totalErrors, |
| b69ab31 | | | 170 | totalDiagnostics, |
| b69ab31 | | | 171 | }, |
| b69ab31 | | | 172 | }); |
| b69ab31 | | | 173 | |
| b69ab31 | | | 174 | if (!enabled) { |
| b69ab31 | | | 175 | return true; |
| b69ab31 | | | 176 | } |
| b69ab31 | | | 177 | |
| b69ab31 | | | 178 | if (totalErrors > 0) { |
| b69ab31 | | | 179 | const buttons = [{label: 'Cancel'}, {label: 'Continue', primary: true}] as const; |
| b69ab31 | | | 180 | const shouldContinue = |
| b69ab31 | | | 181 | (await showModal({ |
| b69ab31 | | | 182 | type: 'confirm', |
| b69ab31 | | | 183 | title: ( |
| b69ab31 | | | 184 | <Row> |
| b69ab31 | | | 185 | <T count={totalErrors}>codeIssuesFound</T> |
| b69ab31 | | | 186 | <Tooltip |
| b69ab31 | | | 187 | title={t( |
| b69ab31 | | | 188 | 'Error-severity issues are typically land blocking and should be resolved before submitting for code review.\n\n' + |
| b69ab31 | | | 189 | 'Errors shown here are best-effort and not necessarily comprehensive.', |
| b69ab31 | | | 190 | )}> |
| b69ab31 | | | 191 | <Icon icon="info" /> |
| b69ab31 | | | 192 | </Tooltip> |
| b69ab31 | | | 193 | </Row> |
| b69ab31 | | | 194 | ), |
| b69ab31 | | | 195 | message: ( |
| b69ab31 | | | 196 | <DiagnosticsList |
| b69ab31 | | | 197 | diagnostics={[...result.diagnostics.entries()]} |
| b69ab31 | | | 198 | tracker={childTracker} |
| b69ab31 | | | 199 | /> |
| b69ab31 | | | 200 | ), |
| b69ab31 | | | 201 | buttons, |
| b69ab31 | | | 202 | })) === buttons[1]; |
| b69ab31 | | | 203 | |
| b69ab31 | | | 204 | childTracker.track('DiagnosticsConfirmationAction', { |
| b69ab31 | | | 205 | extras: { |
| b69ab31 | | | 206 | action: shouldContinue ? 'continue' : 'cancel', |
| b69ab31 | | | 207 | }, |
| b69ab31 | | | 208 | }); |
| b69ab31 | | | 209 | |
| b69ab31 | | | 210 | return shouldContinue; |
| b69ab31 | | | 211 | } |
| b69ab31 | | | 212 | } |
| b69ab31 | | | 213 | } |
| b69ab31 | | | 214 | return true; |
| b69ab31 | | | 215 | } |
| b69ab31 | | | 216 | |
| b69ab31 | | | 217 | function DiagnosticsList({ |
| b69ab31 | | | 218 | diagnostics, |
| b69ab31 | | | 219 | tracker, |
| b69ab31 | | | 220 | }: { |
| b69ab31 | | | 221 | diagnostics: Array<[string, Array<Diagnostic>]>; |
| b69ab31 | | | 222 | tracker: Tracker<{parentId: string}>; |
| b69ab31 | | | 223 | }) { |
| b69ab31 | | | 224 | const [hideNonBlocking, setHideNonBlocking] = useAtom(hideNonBlockingDiagnosticsAtom); |
| b69ab31 | | | 225 | const [shouldWarn, setShouldWarn] = useAtom(shouldWarnAboutDiagnosticsAtom); |
| b69ab31 | | | 226 | return ( |
| b69ab31 | | | 227 | <> |
| b69ab31 | | | 228 | <Column alignStart xstyle={styles.allDiagnostics}> |
| b69ab31 | | | 229 | {diagnostics.map(([filepath, diagnostics]) => { |
| b69ab31 | | | 230 | const sortedDiagnostics = [...diagnostics].filter(d => |
| b69ab31 | | | 231 | hideNonBlocking ? isBlockingDiagnostic(d) : true, |
| b69ab31 | | | 232 | ); |
| b69ab31 | | | 233 | sortedDiagnostics.sort((a, b) => { |
| b69ab31 | | | 234 | return severityComparator(a) - severityComparator(b); |
| b69ab31 | | | 235 | }); |
| b69ab31 | | | 236 | if (sortedDiagnostics.length === 0) { |
| b69ab31 | | | 237 | return null; |
| b69ab31 | | | 238 | } |
| b69ab31 | | | 239 | return ( |
| b69ab31 | | | 240 | <Column key={filepath} alignStart> |
| b69ab31 | | | 241 | <Collapsable |
| b69ab31 | | | 242 | startExpanded |
| b69ab31 | | | 243 | title={ |
| b69ab31 | | | 244 | <Row> |
| b69ab31 | | | 245 | <span>{basename(filepath)}</span> |
| b69ab31 | | | 246 | <Subtle>{filepath}</Subtle> |
| b69ab31 | | | 247 | </Row> |
| b69ab31 | | | 248 | }> |
| b69ab31 | | | 249 | <Column alignStart xstyle={styles.diagnosticList}> |
| b69ab31 | | | 250 | {sortedDiagnostics.map((d, i) => ( |
| b69ab31 | | | 251 | <Row |
| b69ab31 | | | 252 | role="button" |
| b69ab31 | | | 253 | tabIndex={0} |
| b69ab31 | | | 254 | key={i} |
| b69ab31 | | | 255 | xstyle={styles.diagnosticRow} |
| b69ab31 | | | 256 | onClick={() => { |
| b69ab31 | | | 257 | platform.openFile(filepath, {line: d.range.startLine + 1}); |
| b69ab31 | | | 258 | tracker.track('DiagnosticsConfirmationAction', { |
| b69ab31 | | | 259 | extras: {action: 'openFile'}, |
| b69ab31 | | | 260 | }); |
| b69ab31 | | | 261 | }}> |
| b69ab31 | | | 262 | {iconForDiagnostic(d)} |
| b69ab31 | | | 263 | <span>{d.message}</span> |
| b69ab31 | | | 264 | {d.source && ( |
| b69ab31 | | | 265 | <Subtle {...stylex.props(styles.nowrap)}> |
| b69ab31 | | | 266 | {d.source} |
| b69ab31 | | | 267 | {d.code ? `(${d.code})` : null} |
| b69ab31 | | | 268 | </Subtle> |
| b69ab31 | | | 269 | )}{' '} |
| b69ab31 | | | 270 | <Subtle {...stylex.props(styles.nowrap)}> |
| b69ab31 | | | 271 | [Ln {d.range.startLine}, Col {d.range.startCol}] |
| b69ab31 | | | 272 | </Subtle> |
| b69ab31 | | | 273 | </Row> |
| b69ab31 | | | 274 | ))} |
| b69ab31 | | | 275 | </Column> |
| b69ab31 | | | 276 | </Collapsable> |
| b69ab31 | | | 277 | </Column> |
| b69ab31 | | | 278 | ); |
| b69ab31 | | | 279 | })} |
| b69ab31 | | | 280 | </Column> |
| b69ab31 | | | 281 | <Row xstyle={styles.confirmCheckbox}> |
| b69ab31 | | | 282 | <Checkbox |
| b69ab31 | | | 283 | checked={!shouldWarn} |
| b69ab31 | | | 284 | onChange={checked => { |
| b69ab31 | | | 285 | setShouldWarn(!checked); |
| b69ab31 | | | 286 | if (checked) { |
| b69ab31 | | | 287 | tracker.track('DiagnosticsConfirmationAction', {extras: {action: 'dontAskAgain'}}); |
| b69ab31 | | | 288 | } |
| b69ab31 | | | 289 | }}> |
| b69ab31 | | | 290 | <T>Don't ask again</T> |
| b69ab31 | | | 291 | </Checkbox> |
| b69ab31 | | | 292 | <Tooltip |
| b69ab31 | | | 293 | title={t( |
| b69ab31 | | | 294 | "Only 'error' severity issues known to cause problems will cause this dialog to appear, but less severe issues can still be shown here. This option hides these non-blocking issues.", |
| b69ab31 | | | 295 | )}> |
| b69ab31 | | | 296 | <Checkbox checked={hideNonBlocking} onChange={setHideNonBlocking}> |
| b69ab31 | | | 297 | <T>Hide non-blocking issues</T> |
| b69ab31 | | | 298 | </Checkbox> |
| b69ab31 | | | 299 | </Tooltip> |
| b69ab31 | | | 300 | </Row> |
| b69ab31 | | | 301 | </> |
| b69ab31 | | | 302 | ); |
| b69ab31 | | | 303 | } |
| b69ab31 | | | 304 | |
| b69ab31 | | | 305 | function severityComparator(a: Diagnostic) { |
| b69ab31 | | | 306 | switch (a.severity) { |
| b69ab31 | | | 307 | case 'error': |
| b69ab31 | | | 308 | return 0; |
| b69ab31 | | | 309 | case 'warning': |
| b69ab31 | | | 310 | return 1; |
| b69ab31 | | | 311 | case 'info': |
| b69ab31 | | | 312 | return 2; |
| b69ab31 | | | 313 | case 'hint': |
| b69ab31 | | | 314 | return 3; |
| b69ab31 | | | 315 | } |
| b69ab31 | | | 316 | } |
| b69ab31 | | | 317 | |
| b69ab31 | | | 318 | function iconForDiagnostic(d: Diagnostic): React.ReactNode { |
| b69ab31 | | | 319 | switch (d.severity) { |
| b69ab31 | | | 320 | case 'error': |
| b69ab31 | | | 321 | return <Icon icon="error" color="red" />; |
| b69ab31 | | | 322 | case 'warning': |
| b69ab31 | | | 323 | return <Icon icon="warning" color="yellow" />; |
| b69ab31 | | | 324 | case 'info': |
| b69ab31 | | | 325 | return <Icon icon="info" />; |
| b69ab31 | | | 326 | case 'hint': |
| b69ab31 | | | 327 | return <Icon icon="info" />; |
| b69ab31 | | | 328 | } |
| b69ab31 | | | 329 | } |