diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c7cb0e9..2da0cf84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- A method call on a variable is no longer guessed to be a test double's method just because the double has the only method of that name. That covered classes named `Mocked…`, `Fake…`, `Stub…` or `Dummy…`: django-allauth's `resp.json()` calls had all gone to its `MockedResponse`. A test that names the double still calls it. - Kotlin imports now decide what a name means, as Java imports already do. `import java.net.Proxy` or `import org.mockito.Mockito.mock` no longer links to a same-named project symbol, and a member the file declares itself still comes first. An `import x.Foo` picks the right one of several same-named classes: on okhttp, `MockResponse.Builder()` and `Request.Builder()` now reach their classes. A renaming import like `import …internal.unsafeFlow as flow` means the function it names. - In Java, a class you import from outside your project now owns its name. After `import java.lang.reflect.Field;` the file's `Field` no longer links to a project class that happens to share the name. In gson that was a test's nested `Field` class; in jsoup's W3C converter it was jsoup's own `Document` and `Element`. A static import like `import static org.mockito.Mockito.verify;` is covered the same way. - Java enum constants and Kotlin enum entries with a body of their own now have their methods in the graph. Examples are `PLUS { int apply(…) { … } }` and `NewBuffer { override fun pipe() … }`. Before, those methods were missing and every call inside them was dropped. jsoup's HTML tokenizer and tree builder are written this way, so over 2,000 of their calls were invisible. Re-index to pick this up. diff --git a/__tests__/test-double-unique-guess.test.ts b/__tests__/test-double-unique-guess.test.ts new file mode 100644 index 00000000..6cb384b0 --- /dev/null +++ b/__tests__/test-double-unique-guess.test.ts @@ -0,0 +1,63 @@ +/** + * A receiver's method guessed from its name alone is never a test double's + * the calling file never names: django-allauth's `resp.json()` on a Django + * test-client response went to its `MockedResponse.json` 193 times, as the + * one `json` method in the project. A test that names the double keeps it. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-test-double-')); + const files: Record = { + 'tests/mocking.py': `class MockedResponse: + def __init__(self, data): + self.data = data + + def json(self): + return self.data +`, + 'tests/test_api.py': `def test_profile(client): + resp = client.get("/profile") + assert resp.json()["ok"] +`, + 'tests/test_mocking.py': `from tests.mocking import MockedResponse + + +def test_mocked(): + resp = MockedResponse({"ok": True}) + assert resp.json()["ok"] +`, + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +const callsTo = (file: string, name: string) => { + const ids = cg.getNodesInFile(file).map((n) => n.id); + return cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind === 'calls' && cg.getNode(e.target)!.name === name).length; +}; + +describe('test doubles as guessed receivers', () => { + it('are not what an unrelated `resp.json()` calls', () => { + expect(callsTo('tests/test_api.py', 'json')).toBe(0); + }); + + it('are what a test that names them calls', () => { + expect(callsTo('tests/test_mocking.py', 'json')).toBe(1); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 8098681a..1fce2076 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1932,6 +1932,16 @@ const DISPATCHED_ACTIONS: ReadonlySet = new Set([ /** A test double's name: Fake…, Mock…, Stub…, Dummy…, Spy…, …Fake, …Mock, …Stub. */ const TEST_DOUBLE_OWNER = /\b(?:fake|mock|mocked|stub|dummy|spy)\b/i; +/** A method of a test double (`MockedResponse`, `_FakeHTTPResponse`) the receiver and the calling file never name. */ +function isUnnamedTestDouble(method: Node, receiver: string, ref: UnresolvedRef, context: ResolutionContext): boolean { + const cut = method.qualifiedName.lastIndexOf('::'); + if (cut <= 0) return false; + const owner = method.qualifiedName.slice(0, cut).split(/::|\./).pop()!; + if (!TEST_DOUBLE_OWNER.test(splitCamelCase(owner).join(' '))) return false; + if (splitCamelCase(receiverLink(receiver)).some((w) => TEST_DOUBLE_OWNER.test(w))) return false; + return !(context.readFile(ref.filePath) ?? '').includes(owner); +} + /** * The link of a dotted receiver its value is named after: the last, or for a * constant (`InitializationPhase.CONTROLLERS`, `Foo.INSTANCE`) the type it @@ -6383,6 +6393,10 @@ export function matchMethodCall( // `node.loc` on a rubocop-ast node went to the one `loc` in the project // 1,201 times; lobsters' `value.to_s` to a short-id class's. if (targetMethods.length === 1 && !narrowed && targetMethods[0]!.language === ref.language && + // A test double is only what a test names — as in the scoring below: + // allauth's `resp.json()` on a Django test response is not the one + // `json` of its `MockedResponse`. + !isUnnamedTestDouble(targetMethods[0]!, objectOrClass!, ref, context) && !((ref.language === 'lua' || ref.language === 'luau') && isLuaLibraryCall(objectOrClass!, methodName!, ref, targetMethods[0]!)) && // Rust / Go / Kotlin / C#: a standard-library method name on an // untyped receiver (`sym.map(…)`, `w.Header().Get(…)`,