From 2afe05a23df5a53248085f3fb574c996e8ce378e Mon Sep 17 00:00:00 2001 From: mayanour4 <113398919+mayanour4@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:35:00 -0400 Subject: [PATCH] Add source location mapping for test smell diagnostics --- .vscodeignore | 1 + detector/README.md | 5 + detector/tsdetect-locations.jar | Bin 0 -> 3187 bytes java/LocationBridge.java | 177 ++++++++++++++++++++++++++++++++ src/extension.ts | 87 +++++++++++++--- src/testSmellDetector.ts | 153 ++++++++++++++++++--------- 6 files changed, 363 insertions(+), 60 deletions(-) create mode 100644 detector/tsdetect-locations.jar create mode 100644 java/LocationBridge.java diff --git a/.vscodeignore b/.vscodeignore index c28bc23..30a60eb 100644 --- a/.vscodeignore +++ b/.vscodeignore @@ -2,6 +2,7 @@ .github/** src/** scripts/** +java/** regression-corpus/** .gitignore tsconfig.json diff --git a/detector/README.md b/detector/README.md index 37ff55b..8dfc634 100644 --- a/detector/README.md +++ b/detector/README.md @@ -8,3 +8,8 @@ It was built as the Maven `jar-with-dependencies` artifact using Java 11. The detector is licensed separately under GNU GPL version 3; see `LICENSE` in this directory. Review the GPL source-distribution requirements before publishing or distributing an extension package that contains this JAR. + +`tsdetect-locations.jar` is the extension's small integration bridge. It calls +the bundled detector API, maps engine-confirmed method and class names to +JavaParser declaration ranges, and returns those findings to the TypeScript +adapter. Its source is kept in `java/LocationBridge.java`. diff --git a/detector/tsdetect-locations.jar b/detector/tsdetect-locations.jar new file mode 100644 index 0000000000000000000000000000000000000000..a2fe41deb2cb4602bdea6336960bd3b5b1fd8eda GIT binary patch literal 3187 zcmZ{nc{mi>8^_0F9jP#cToSU4B|{;~zK_P5tu$sVnKXmJluX&TD9XN$HCv2JF=QJQ z#@KR8Gl?Qe8ABLx_0#ive)sm<-t(O2J?DA8@B4nwf1d-lIlu`50C)iay#TjMfM0?S zzzKkvIT@*1*_o^F4FLe~eJL2g%gg&;Qt*D+Us9Nnot3$nqmvrUd>uv|wzW}J8yB!q zRi;r#e|OM$37uUCR+~=;qiATjf0%3E4i9fix(WKHY0v%rw>_Qzc*#$DwiqM= zcNKHpIQS~c&qob;4G|Jje~}OGG$pc!@xAdNJ3H{myi&lyWJHFBm>Hs59z`0LF*?Z^ zV{%uTC`7LiEF(m)*@6JH2 z;^joU8oWM6v~4jWXxq%`l{WRx9rk5F6(}JY_!|6LTW1!mNJm{e)gdVRW;t+q2F2|N zq&d&|>x7c;)CUA50J9Aj<}0%-`@U$O*hreeb&12Y1vp3W%91JM5>TDLM51G?c#Xz; zmZgsWi_E;_hq<;>PJO~8xLyuXY#eR|DNL?%6bX`@%JwWZjyfqL!cVPVyYQL6Txiap zY0X!zbvV$M$h&}5$tj!Dpo(BUmdCMFx^P;~Ov^)6_>pBnSKXMgb-%F>#cGI(Xo`}1 z(TQn{`H7jZw7j+&Zz&w#cV4MT?(GEQbRLi6SXAe?JCfRYN)pg4IJUVv+R3ZecC-Hk zvHa=yms1nzPCJ}t{q~L3IzrE_wK9PgPTN&qiW!!n!So=BwC5){R8iH9&GKCO^Lb&G zc_{AEXyqo4Dq^+ffDT7qa==T)M^lZQm!p!e)V-r0v)ia>e-?~kSjvsWZe2U{%H4z` z!7TsV1s91H8Wk<)I1}%BTVIkCjW&?YykZA-)!W%yarGZ-AF$o%H%|4{@RozPh$zgV zxQ4X~dONz`oBW{wpFmf-BW0j%Hya~0?7s0=-RzR_mV3}Oj(390&7HSyW-yFcLXW+wiS`1D!F-uIJ) zK)t_kjzVG93iGsZpn+x@lq0=)$TKBbsGt_{8xS}oDw}bdeiJ1h;HGcj<9rLStc|-8 za|d19*P`~s`N}wo)31l-E&kRJ?@;n%0G3s>^ zP(64hbCzSh)uRb#&3mG}`{h{7jAs&+-YdH$KIjs1+(#T)HqVQG9XkTtd}Ma6E}%w6 z^9)zvxH^`N3Vp=1s}wSSQuHYCc%_V5C|QvZoilcy(=2O!RWztk$$O~XG~Rhe5Ll`I zL-YpW%~KtWQO}j46@4pZZftJ+VRte`d@9CaYeC|1cV?ZZ?%R6vt{baMm95(W0@4H8 z3`|>_uB1Sa*=*_6$3?`KqtSs2JyPfJovoBO^i_PWu$rejAZeaEY2G|)mH6l!c<4)< zrORm*IA*{?gi$(&Y_eAzyf|zue77&7GWyh@9R zlReImel>w`oAin%PPj{?CXy?6V=I6SIrseW^A5Sd7jjhe`KI(vd|9##qiU8Q;MIMA z^GtPs!MH{+W$=)!&>@(zsH@9{luOaJLz9+jzllU=VnS}VTQ{)9nHegZcYqap=E%xD z3){B0H4^RG`pSoW%UBD$rv$vRLdS{@dJT0;%%$(BZh!{-r*F~S!gG=rUdx|9+_oCK zfEB8Eq@b-I7YT;pCu`m* z>Q>j@wFWlkj^DTK_d??jbjcjA1*aFg_*LLjPAFeu|3+sFS&NU}9$U58OOA z(*`;|#%5#g5U-oZd>oqo1a+b0vySpb3EZ>_p_8>YzFztbwhZj>Uus?2$^9lk^3V8c zRbz6wij!e@56bhWe)vbJ>)CFjaRd{0{N<^;Tk>3_LB8vkk$v@lg7zr>g^1cadMpVm zW&D{22&_)A`C?d9OI<~vw^514k(E@5YFu}hEsuC*k~q4Soypb6|93KU49d$Sd~ z>{9R$5KDbG{~*J?G47fVV|MF{^NKM`O0jUIR}gHlM4H+**d%L9SzM}p?&cQ(lrK|U zpWeE1*)J6KmG|S0WCRwqC?z?i8;vxF6VJwz#ugbd!g^@)Mm$)mxKw4Og7Rp+O~+M@ zCyxB5JzMcB)xK`Cn*6zB)-HeNi(%EJ@I#dI$t_X#&BLr4$X3Jdi{(dCi5{z^ZPsaQ z%UdA9;!5;rYF9OylDHBZ;=k)mSLzQA8x)&BDUaJU*$&UY@RnH3{9KVyjT6iE&G~|I zkf$enb9h^EH0^y2k+rxe%8Wg06OIY-QZQ$w6IsK!4PD5@6oR1@TgI0%NojIS#V612 zd2Ey_gik*4=rbmP>HXE-nL05~(NPOgwe~&Cpl^MwW^w{^Hh^j&AX*~oy!sxsr|m87 z&ZaJPgK!9u@9@zlHR5#3XGuRWQPS-s?ucn6CZ!D@pSpUCS3~kmHS)mb2YvfQJ0j_- zY88hHHr+K*tyI^sfHHtPFX+ive@*g?q2ub4=nO`KXOI4Jct$jT{kstm-A1yT1v)jVgVHkC|VFHD!69jl=q7QGCWxpl*w{lRU{jTjMZ z?3D+uY3=ImF3!nXwxf$}ioypbO&a-&haKxx$R^>ZYQj9WRZNhw#S(@&L=s=c>$|$2 z{IV==5TDN?{RpCJ#K{MY?2i?laR^eTN{r;Cq0XE9%h-nm(N&7 z5?VF8;f`LKHP}6^B!)Yfpfnh{^vQ8)Z0&&JLfULs3k~j3*Zm_;L!Vz)SP&%_S z6?RouV+Iqw@#f*I<_uR);xD5vo1o;1az7Mv)#9s*hPg>^yf&qW?`md<` y6Nvu~mHp#irN7Yo&*A^-@Dr(jHTLfv_CE*z6R_bnT--nB9N1r7`?-k!=j|UOW4v|% literal 0 HcmV?d00001 diff --git a/java/LocationBridge.java b/java/LocationBridge.java new file mode 100644 index 0000000..61219f9 --- /dev/null +++ b/java/LocationBridge.java @@ -0,0 +1,177 @@ +import com.github.javaparser.JavaParser; +import com.github.javaparser.Range; +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.body.MethodDeclaration; +import com.github.javaparser.ast.body.TypeDeclaration; +import java.io.File; +import java.io.IOException; +import java.io.Writer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import testsmell.AbstractSmell; +import testsmell.SmellyElement; +import testsmell.TestClass; +import testsmell.TestFile; +import testsmell.TestMethod; +import testsmell.TestSmellDetector; +import testsmell.Util; +import thresholds.DefaultThresholds; + +/** + * Runs tsDetect and maps its smelly method or class names to JavaParser source + * ranges. Ambiguous elements remain file-level findings instead of being + * assigned to a declaration that may be incorrect. + */ +public final class LocationBridge { + private static final String FORMAT_VERSION = "TSDETECT_LOCATIONS_V1"; + + private LocationBridge() {} + + public static void main(String[] arguments) throws Exception { + if (arguments.length != 4) { + throw new IllegalArgumentException( + "Expected application, test, production, and output paths." + ); + } + + String applicationName = arguments[0]; + String testFilePath = arguments[1]; + String productionFilePath = arguments[2]; + Path outputPath = Path.of(arguments[3]); + + TestFile testFile = new TestFile( + applicationName, + testFilePath, + productionFilePath + ); + TestFile result = new TestSmellDetector(new DefaultThresholds()) + .detectSmells(testFile); + CompilationUnit compilationUnit = JavaParser.parse( + new File(testFilePath) + ); + List methods = compilationUnit.getNodesByType( + MethodDeclaration.class + ); + List types = compilationUnit.getNodesByType( + TypeDeclaration.class + ); + + try ( + Writer output = Files.newBufferedWriter( + outputPath, + StandardCharsets.UTF_8 + ) + ) { + writeRow( + output, + FORMAT_VERSION, + String.valueOf(result.getNumberOfTestMethods()) + ); + + for (AbstractSmell smell : result.getTestSmells()) { + if (smell == null || !smell.hasSmell()) { + continue; + } + + boolean wroteFinding = false; + for (SmellyElement element : smell.getSmellyElements()) { + if (element == null || !element.isSmelly()) { + continue; + } + + wroteFinding = true; + writeFinding(output, smell.getSmellName(), element, methods, types); + } + + if (!wroteFinding) { + writeFinding(output, smell.getSmellName(), "file", "", null); + } + } + } + } + + private static void writeFinding( + Writer output, + String smellName, + SmellyElement element, + List methods, + List types + ) throws IOException { + String elementName = element.getElementName(); + List matches = new ArrayList<>(); + String scope = "file"; + + if (element instanceof TestMethod) { + for (MethodDeclaration method : methods) { + if ( + method.getNameAsString().equals(elementName) + && Util.isValidTestMethod(method) + ) { + matches.add(method.getName()); + } + } + if (matches.size() == 1) { + scope = "method"; + } + } else if (element instanceof TestClass) { + for (TypeDeclaration type : types) { + if (type.getNameAsString().equals(elementName)) { + matches.add(type.getName()); + } + } + if (matches.size() == 1) { + scope = "class"; + } + } + + Node matchedNode = matches.size() == 1 ? matches.get(0) : null; + writeFinding(output, smellName, scope, elementName, matchedNode); + } + + private static void writeFinding( + Writer output, + String smellName, + String scope, + String elementName, + Node node + ) throws IOException { + Optional range = node == null + ? Optional.empty() + : node.getRange(); + + if (range.isEmpty()) { + writeRow(output, smellName, "file", elementName, "", "", "", ""); + return; + } + + Range sourceRange = range.get(); + writeRow( + output, + smellName, + scope, + elementName, + String.valueOf(sourceRange.begin.line), + String.valueOf(sourceRange.begin.column), + String.valueOf(sourceRange.end.line), + String.valueOf(sourceRange.end.column + 1) + ); + } + + private static void writeRow(Writer output, String... values) + throws IOException { + for (int index = 0; index < values.length; index += 1) { + if (index > 0) { + output.write(','); + } + output.write('"'); + output.write(values[index].replace("\"", "\"\"")); + output.write('"'); + } + output.write('\n'); + } +} diff --git a/src/extension.ts b/src/extension.ts index d33e6ea..9ebc2b4 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -6,7 +6,7 @@ import { isLikelyJavaTestDocument, ProductionFileResolver, } from './productionFileResolver'; -import { runDetector } from './testSmellDetector'; +import { DetectorFinding, runDetector } from './testSmellDetector'; const ANALYZE_CURRENT_FILE_COMMAND = 'testSmellDetector.analyzeCurrentFile'; const DIAGNOSTIC_SOURCE = 'Test Smell Detector'; @@ -18,9 +18,8 @@ interface AnalysisRequest { } /** - * tsDetect currently reports smells for the complete test file rather than an - * exact source line. Anchor diagnostics to the declared test class so VS Code - * can show an honest file-level result in the Problems panel. + * Find the declared test class for findings that do not have one unambiguous + * method or class location. */ function findTestClassRange(document: vscode.TextDocument): vscode.Range { const source = document.getText(); @@ -42,21 +41,71 @@ function findTestClassRange(document: vscode.TextDocument): vscode.Range { ); } -/** Convert file-level smell names into native VS Code warning diagnostics. */ +/** Convert a one-based bridge location into a validated VS Code range. */ +function getFindingRange( + document: vscode.TextDocument, + finding: DetectorFinding, +): vscode.Range | undefined { + const location = finding.location; + if (!location) { + return undefined; + } + + const startLine = location.startLine - 1; + const startCharacter = location.startColumn - 1; + const endLine = location.endLine - 1; + const endCharacter = location.endColumn - 1; + + if ( + startLine < 0 || + endLine < startLine || + endLine >= document.lineCount || + startCharacter < 0 || + endCharacter < 0 || + startCharacter > document.lineAt(startLine).text.length || + endCharacter > document.lineAt(endLine).text.length || + (startLine === endLine && endCharacter <= startCharacter) + ) { + return undefined; + } + + return new vscode.Range( + startLine, + startCharacter, + endLine, + endCharacter, + ); +} + +/** Convert engine findings into native VS Code warning diagnostics. */ function createDiagnostics( document: vscode.TextDocument, + findings: DetectorFinding[], detectedSmells: string[], ): vscode.Diagnostic[] { - const range = findTestClassRange(document); - - return detectedSmells.map((smell) => { + const fallbackRange = findTestClassRange(document); + const displayFindings: DetectorFinding[] = + findings.length > 0 + ? findings + : detectedSmells.map((name) => ({ + name, + scope: 'file', + elementName: '', + })); + + return displayFindings.map((finding) => { + const mappedRange = getFindingRange(document, finding); + const range = mappedRange ?? fallbackRange; + const message = mappedRange + ? `${finding.name} detected in ${finding.scope} ${finding.elementName}. The underline marks the affected declaration.` + : `${finding.name} detected in this test file (exact source location unavailable).`; const diagnostic = new vscode.Diagnostic( range, - `${smell} detected in this test file (file-level result).`, + message, vscode.DiagnosticSeverity.Warning, ); diagnostic.source = DIAGNOSTIC_SOURCE; - diagnostic.code = smell; + diagnostic.code = finding.name; return diagnostic; }); } @@ -214,7 +263,11 @@ export function activate(context: vscode.ExtensionContext): void { } else { diagnostics.set( document.uri, - createDiagnostics(document, result.detectedSmells), + createDiagnostics( + document, + result.findings, + result.detectedSmells, + ), ); } @@ -240,10 +293,18 @@ export function activate(context: vscode.ExtensionContext): void { for (const smell of result.detectedSmells) { output.appendLine(`- ${smell}`); } + output.appendLine(''); + output.appendLine(`Mapped findings (${result.findings.length}):`); + for (const finding of result.findings) { + const mappedElement = finding.location + ? `${finding.scope} ${finding.elementName} at line ${finding.location.startLine}` + : 'file level (exact declaration unavailable)'; + output.appendLine(`- ${finding.name}: ${mappedElement}`); + } output.show(true); void vscode.window .showWarningMessage( - `${result.detectedSmells.length} test smell(s) detected in ${fileName}.`, + `${result.findings.length} test smell finding(s) detected in ${fileName}.`, 'Open Problems', ) .then((selection) => { @@ -256,7 +317,7 @@ export function activate(context: vscode.ExtensionContext): void { } } else { reportAutomaticMessage( - `${fileName}: ${result.numberOfTestMethods} test method(s), ${result.detectedSmells.length} smell(s).`, + `${fileName}: ${result.numberOfTestMethods} test method(s), ${result.findings.length} smell finding(s).`, ); } } catch (error: unknown) { diff --git a/src/testSmellDetector.ts b/src/testSmellDetector.ts index 012a900..b61acaa 100644 --- a/src/testSmellDetector.ts +++ b/src/testSmellDetector.ts @@ -1,16 +1,13 @@ import { execFile } from 'node:child_process'; -import { access, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { access, mkdtemp, readFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import * as path from 'node:path'; import { promisify } from 'node:util'; -import { encodeCsvRow, parseCsv } from './csv'; +import { parseCsv } from './csv'; const execFileAsync = promisify(execFile); - -// Smell columns follow seven metadata columns: app, class, absolute paths, -// relative paths, and the number of test methods. -const RESULT_METADATA_COLUMNS = 7; +const LOCATION_FORMAT_VERSION = 'TSDETECT_LOCATIONS_V1'; /** All filesystem and runtime inputs required for one detector invocation. */ export interface DetectorOptions { @@ -21,42 +18,60 @@ export interface DetectorOptions { testFilePath: string; } -/** The small, editor-facing subset of tsDetect's complete CSV result. */ +/** One-based Java source positions; the ending column is exclusive. */ +export interface DetectorLocation { + startLine: number; + startColumn: number; + endLine: number; + endColumn: number; +} + +/** One engine-confirmed smell mapped to a method, class, or file. */ +export interface DetectorFinding { + name: string; + scope: 'method' | 'class' | 'file'; + elementName: string; + location?: DetectorLocation; +} + +/** The editor-facing subset of tsDetect's results. */ export interface DetectorResult { detectedSmells: string[]; + findings: DetectorFinding[]; numberOfTestMethods: number; } /** - * Runs tsDetect for one test/production pair and converts its CSV output into - * a result that the VS Code layer can display. + * Runs tsDetect through the location bridge and converts its normalized CSV + * output into findings that VS Code can place on source declarations. */ export async function runDetector(options: DetectorOptions): Promise { - // Fail before creating temporary files if the bundled detector is missing. + const bridgePath = path.join( + path.dirname(options.jarPath), + 'tsdetect-locations.jar', + ); + + // Fail before creating temporary files if either bundled JAR is missing. await access(options.jarPath); + await access(bridgePath); - // A unique directory prevents concurrent runs from sharing CSV files. const temporaryDirectory = await mkdtemp( path.join(tmpdir(), 'test-smell-detector-'), ); - const inputPath = path.join(temporaryDirectory, 'input.csv'); - const outputPath = path.join(temporaryDirectory, 'output.csv'); + const outputPath = path.join(temporaryDirectory, 'locations.csv'); try { - // The existing CLI expects one headerless row containing the application, - // test-file path, and production-file path. - const inputRow = encodeCsvRow([ - options.applicationName, - options.testFilePath, - options.productionFilePath, - ]); - await writeFile(inputPath, `${inputRow}\n`, 'utf8'); - - // execFile passes an argument array directly to Java and intentionally - // avoids interpreting source paths through a command shell. await execFileAsync( options.javaPath, - ['-jar', options.jarPath, '-f', inputPath, '-o', outputPath], + [ + '-cp', + [bridgePath, options.jarPath].join(path.delimiter), + 'LocationBridge', + options.applicationName, + options.testFilePath, + options.productionFilePath, + outputPath, + ], { encoding: 'utf8', maxBuffer: 4 * 1024 * 1024, @@ -64,43 +79,87 @@ export async function runDetector(options: DetectorOptions): Promise isDetected(values[index + RESULT_METADATA_COLUMNS])); - const numberOfTestMethods = Number.parseInt(values[6], 10); + const parsedFindings = rows.slice(1).map(parseFinding); + const findings = [ + ...new Map( + parsedFindings.map((finding) => [findingKey(finding), finding]), + ).values(), + ]; + const detectedSmells = [...new Set(findings.map((finding) => finding.name))]; return { detectedSmells, - numberOfTestMethods: Number.isNaN(numberOfTestMethods) - ? 0 - : numberOfTestMethods, + findings, + numberOfTestMethods, }; } finally { - // Temporary inputs and outputs are removed after successful and failed runs. await rm(temporaryDirectory, { force: true, recursive: true }); } } -function isDetected(value: string): boolean { - const normalizedValue = value.trim().toLowerCase(); +function findingKey(finding: DetectorFinding): string { + const location = finding.location; + return [ + finding.name, + finding.scope, + finding.elementName, + location?.startLine ?? '', + location?.startColumn ?? '', + location?.endLine ?? '', + location?.endColumn ?? '', + ].join('\u0000'); +} + +function parseFinding(row: string[]): DetectorFinding { + if ( + row.length !== 7 || + !row[0] || + !['method', 'class', 'file'].includes(row[1]) + ) { + throw new Error('tsDetect returned an invalid source finding.'); + } + + const scope = row[1] as DetectorFinding['scope']; + const finding: DetectorFinding = { + name: row[0], + scope, + elementName: row[2], + }; + + if (scope === 'file') { + return finding; + } + + const [startLine, startColumn, endLine, endColumn] = row + .slice(3) + .map(Number); + if ( + ![startLine, startColumn, endLine, endColumn].every( + (value) => Number.isSafeInteger(value) && value > 0, + ) || + endLine < startLine || + (endLine === startLine && endColumn <= startColumn) + ) { + throw new Error('tsDetect returned an invalid source location.'); + } - // tsDetect supports boolean and numerical result granularities. - return normalizedValue === 'true' || Number(normalizedValue) > 0; + finding.location = { startLine, startColumn, endLine, endColumn }; + return finding; }