fix: fix issue with stale cache entries (#2673)

* fix spelling
* fix: Fix the stale cache issue
* Add Object collector class

packages/cspell-gitignore/src/GitIgnore.ts

@@ -97,11 +97,11 @@ export class GitIgnore {
         const parent = path.dirname(directory);
         const parentHierarchy =
             parent !== directory && contains(root, parent) ? await this.findGitIgnoreHierarchy(parent) : undefined;
-        const gif = await loadGitIgnore(directory);
-        if (!gif) {
+        const git = await loadGitIgnore(directory);
+        if (!git) {
             return parentHierarchy || new GitIgnoreHierarchy([]);
         }
-        const chain = parentHierarchy?.gitIgnoreChain.concat([gif]) ?? [gif];
+        const chain = parentHierarchy?.gitIgnoreChain.concat([git]) ?? [git];
         return new GitIgnoreHierarchy(chain);
     }
 

packages/cspell-gitignore/src/GitIgnoreFile.ts

@@ -58,8 +58,8 @@ export class GitIgnoreHierarchy {
     }
 
     isIgnored(file: string): boolean {
-        for (const gif of this.gitIgnoreChain) {
-            if (gif.isIgnored(file)) return true;
+        for (const git of this.gitIgnoreChain) {
+            if (git.isIgnored(file)) return true;
         }
 
         return false;
@@ -71,8 +71,8 @@ export class GitIgnoreHierarchy {
      * @returns IsIgnoredExResult of the match or undefined if there was no match.
      */
     isIgnoredEx(file: string): IsIgnoredExResult | undefined {
-        for (const gif of this.gitIgnoreChain) {
-            const r = gif.isIgnoredEx(file);
+        for (const git of this.gitIgnoreChain) {
+            const r = git.isIgnoredEx(file);
             if (r.matched) return r;
         }
 

packages/cspell/src/util/cache/DiskCache.test.ts

@@ -1,7 +1,9 @@
 import * as FileEntryCacheModule from 'file-entry-cache';
 import * as path from 'path';
 import * as fileHelper from '../../util/fileHelper';
-import { CachedFileResult, DiskCache, CSpellCacheMeta } from './DiskCache';
+import { CachedFileResult, DiskCache, CSpellCacheMeta, __testing__ } from './DiskCache';
+
+const { calcVersion } = __testing__;
 
 jest.mock('./getConfigHash', () => ({
     getConfigHash: jest.fn().mockReturnValue('TEST_CONFIG_HASH'),
@@ -172,8 +174,8 @@ function entry(result: CachedFileResult, dependencies: string[] = [], size = 100
             size,
             data: {
                 r: result,
-                d: dependencies,
-                v: 'version',
+                d: dependencies.map((f) => ({ f, h: 'hash' })),
+                v: calcVersion('version'),
             },
         },
     };

packages/cspell/src/util/cache/DiskCache.ts

@@ -1,41 +1,79 @@
+import * as crypto from 'crypto';
 import type { FileDescriptor, FileEntryCache } from 'file-entry-cache';
 import * as fileEntryCache from 'file-entry-cache';
+import * as fs from 'fs';
 import { resolve as resolvePath } from 'path';
 import type { FileResult } from '../../util/fileHelper';
 import { readFileInfo } from '../../util/fileHelper';
 import type { CSpellLintResultCache } from './CSpellLintResultCache';
+import { ShallowObjectCollection } from './ObjectCollection';
 
-export type CachedFileResult = Omit<FileResult, 'fileInfo' | 'elapsedTimeMs'>;
+export type CachedFileResult = Omit<FileResult, 'fileInfo' | 'elapsedTimeMs' | 'cached'>;
 
 /**
  * This is the data cached.
  * Property names are short to help keep the cache file size small.
  */
 interface CachedData {
+    /** meta version + suffix */
+    v?: string;
     /** results */
-    r: CachedFileResult;
+    r?: CachedFileResult;
     /** dependencies */
-    d: string[];
-    /** meta version */
-    v: string;
+    d?: Dependency[];
+}
+
+interface Dependency {
+    /** filename */
+    f: string;
+    /** hash of file contents */
+    h?: string | undefined;
 }
 
 interface CSpellCachedMetaData {
     data?: CachedData;
 }
 
-export type CSpellCacheMeta = (FileDescriptor['meta'] & CSpellCachedMetaData) | undefined;
+type Meta = FileDescriptor['meta'];
+
+export type CSpellCacheMeta = (Meta & CSpellCachedMetaData) | undefined;
+
+type CacheDataKeys = {
+    [K in keyof Required<CachedData>]: K;
+};
+
+const cacheDataKeys: CacheDataKeys = {
+    v: 'v',
+    r: 'r',
+    d: 'd',
+};
+
+/**
+ * Meta Data Version is used to detect if the structure of the meta data has changed.
+ * This is used in combination with the Suffix and the version of CSpell.
+ */
+const META_DATA_BASE_VERSION = '1';
+const META_DATA_VERSION_SUFFIX = '-' + META_DATA_BASE_VERSION + '-' + Object.keys(cacheDataKeys).join('|');
+
+interface DependencyCacheTree {
+    d?: Dependency[];
+    c?: Map<string, DependencyCacheTree>;
+}
 
 /**
  * Caches cspell results on disk
  */
 export class DiskCache implements CSpellLintResultCache {
     private fileEntryCache: FileEntryCache;
-    private changedDependencies: Set<string> = new Set();
-    private knownDependencies: Set<string> = new Set();
+    private dependencyCache: Map<string, Dependency> = new Map();
+    private dependencyCacheTree: DependencyCacheTree = {};
+    private objectCollection = new ShallowObjectCollection<CachedData>();
+    private ocCacheFileResult = new ShallowObjectCollection<CachedFileResult>();
+    readonly version: string;
 
-    constructor(cacheFileLocation: string, useCheckSum: boolean, readonly version: string) {
+    constructor(cacheFileLocation: string, readonly useCheckSum: boolean, readonly cspellVersion: string) {
         this.fileEntryCache = fileEntryCache.createFromFile(resolvePath(cacheFileLocation), useCheckSum);
+        this.version = calcVersion(cspellVersion);
     }
 
     public async getCachedLintResults(filename: string): Promise<FileResult | undefined> {
@@ -61,6 +99,14 @@ export class DiskCache implements CSpellLintResultCache {
             return undefined;
         }
 
+        const dd = { ...data };
+
+        if (dd.d) {
+            dd.d = setTreeEntry(this.dependencyCacheTree, dd.d);
+        }
+        dd.r = dd.r && this.normalizeResult(dd.r);
+        meta.data = this.objectCollection.get(dd);
+
         // Skip reading empty files and files without lint error
         const hasErrors = !!result && (result.errors > 0 || result.configErrors > 0 || result.issues.length > 0);
         const cached = true;
@@ -75,7 +121,7 @@ export class DiskCache implements CSpellLintResultCache {
     }
 
     public setCachedLintResults(
-        { fileInfo, elapsedTimeMs: _, ...result }: FileResult,
+        { fileInfo, elapsedTimeMs: _, cached: __, ...result }: FileResult,
         dependsUponFiles: string[]
     ): void {
         const fileDescriptor = this.fileEntryCache.getFileDescriptor(fileInfo.filename);
@@ -84,37 +130,128 @@ export class DiskCache implements CSpellLintResultCache {
             return;
         }
 
-        const data: CachedData = {
-            r: result,
-            d: dependsUponFiles,
+        const data: CachedData = this.objectCollection.get({
             v: this.version,
-        };
+            r: this.normalizeResult(result),
+            d: this.calcDependencyHashes(dependsUponFiles),
+        });
 
         meta.data = data;
-        this.cacheDependencies(dependsUponFiles);
     }
 
     public reconcile(): void {
         this.fileEntryCache.reconcile();
     }
 
-    private cacheDependencies(files: string[]) {
-        this.fileEntryCache.analyzeFiles(files);
+    private normalizeResult(result: CachedFileResult): CachedFileResult {
+        const { issues, processed, errors, configErrors, ...rest } = result;
+        if (!Object.keys(rest).length) {
+            return this.ocCacheFileResult.get(result);
+        }
+        return this.ocCacheFileResult.get({ issues, processed, errors, configErrors });
+    }
+
+    private calcDependencyHashes(dependsUponFiles: string[]): Dependency[] {
+        dependsUponFiles.sort();
+
+        const c = getTreeEntry(this.dependencyCacheTree, dependsUponFiles);
+        if (c?.d) {
+            return c.d;
+        }
+
+        const dependencies: Dependency[] = dependsUponFiles.map((f) => this.getDependency(f));
+
+        return setTreeEntry(this.dependencyCacheTree, dependencies);
+    }
+
+    private checkDependency(dep: Dependency): boolean {
+        const cDep = this.dependencyCache.get(dep.f);
+
+        if (cDep && compDep(dep, cDep)) return true;
+        if (cDep) return false;
+
+        const d = this.getFileDep(dep.f);
+        if (compDep(dep, d)) {
+            this.dependencyCache.set(dep.f, dep);
+            return true;
+        }
+        this.dependencyCache.set(d.f, d);
+        return false;
+    }
+
+    private getDependency(file: string): Dependency {
+        const dep = this.dependencyCache.get(file);
+        if (dep) return dep;
+        const d = this.getFileDep(file);
+        this.dependencyCache.set(file, d);
+        return d;
+    }
+
+    private getFileDep(file: string): Dependency {
+        let h: string;
+        try {
+            const buffer = fs.readFileSync(file);
+            h = this.getHash(buffer);
+        } catch (e) {
+            return { f: file };
+        }
+        return { f: file, h };
     }
 
-    private checkDependencies(files: string[]): boolean {
-        for (const file of files) {
-            if (this.changedDependencies.has(file)) {
+    private checkDependencies(dependencies: Dependency[] | undefined): boolean {
+        if (!dependencies) return false;
+        for (const dep of dependencies) {
+            if (!this.checkDependency(dep)) {
                 return false;
             }
         }
-        const unknown = files.filter((f) => !this.knownDependencies.has(f));
-        if (!unknown.length) {
-            return true;
+        return true;
+    }
+
+    private getHash(buffer: Buffer): string {
+        return crypto.createHash('md5').update(buffer).digest('hex');
+    }
+}
+
+function getTreeEntry(tree: DependencyCacheTree, keys: string[]): DependencyCacheTree | undefined {
+    let r: DependencyCacheTree | undefined = tree;
+    for (const k of keys) {
+        r = r.c?.get(k);
+        if (!r) return r;
+    }
+    return r;
+}
+
+function setTreeEntry(tree: DependencyCacheTree, deps: Dependency[], update = false): Dependency[] {
+    let r = tree;
+    for (const d of deps) {
+        const k = d.f;
+        if (!r.c) {
+            r.c = new Map();
+        }
+        const cn = r.c.get(k);
+        const n = cn ?? {};
+        if (!cn) {
+            r.c.set(k, n);
         }
-        const { changedFiles, notFoundFiles } = this.fileEntryCache.analyzeFiles(files);
-        changedFiles.map((f) => this.changedDependencies.add(f));
-        unknown.forEach((f) => this.knownDependencies.add(f));
-        return changedFiles.length === 0 && notFoundFiles.length === 0;
+        r = n;
     }
+    let d = r.d;
+    if (!d || (r.d && update)) {
+        r.d = deps;
+        d = deps;
+    }
+    return d;
+}
+
+function compDep(a: Dependency, b: Dependency) {
+    return a.f === b.f && a.h === b.h;
 }
+
+function calcVersion(version: string): string {
+    return version + META_DATA_VERSION_SUFFIX;
+}
+
+export const __testing__ = {
+    calcVersion,
+};

packages/cspell/src/util/cache/ObjectCollection.test.ts

@@ -0,0 +1,57 @@
+import { ShallowObjectCollection, Collection } from './ObjectCollection';
+
+const objects = (function () {
+    const empty = {};
+    const a = { name: 'a' };
+    const b = { name: 'b' };
+    const bb = { name: 'b', value: 42 };
+    const c = { kind: 'c', name: 'a' };
+    const n = { kind: 'c', a };
+    const ab = { a: a, b: b };
+    const arr = ['hello', a, b, c];
+    const cir = { values: [] };
+    (cir.values as unknown[]).push(cir);
+    return { empty, a, b, bb, c, n, ab, arr, cir };
+})();
+
+describe('ObjectCollection', () => {
+    const { empty, a, b, bb, c, n, ab } = objects;
+    test.each`
+        objects                                                                    | expected
+        ${[a, b, { ...a }, bb, c, n, empty, {}]}                                   | ${[a, b, a, bb, c, n, empty, empty]}
+        ${['', 0, a, 42, empty, c, 'hello', {}, { ...c }]}                         | ${['', 0, a, 42, empty, c, 'hello', empty, c]}
+        ${[a, b, n, ab, { a: { ...a }, b: { ...b } }, { kind: 'c', a: { ...a } }]} | ${[a, b, n, ab, ab, n]}
+    `('ShallowObjectCollection $objects', ({ objects, expected }: { objects: object[]; expected: unknown[] }) => {
+        const c = new ShallowObjectCollection();
+        const r = objects.map((v) => c.get(v));
+        expect(r).toEqual(expected);
+    });
+});
+
+describe('Collection', () => {
+    const { empty, a, b, bb, c, n, ab, arr, cir } = objects;
+    const sym = Symbol('test');
+    const f = Object.freeze({ ...a });
+    const nestedCircular = { cir };
+
+    test.each`
+        objects                                                                    | expected                                       | comment
+        ${[a, b, { ...a }, bb, c, n, empty, {}]}                                   | ${[a, b, a, bb, c, n, empty, empty]}           | ${''}
+        ${['', 0, a, 42, empty, c, 'hello', {}, { ...c }]}                         | ${['', 0, a, 42, empty, c, 'hello', empty, c]} | ${''}
+        ${[a, b, n, ab, { a: { ...a }, b: { ...b } }, { kind: 'c', a: { ...a } }]} | ${[a, b, n, ab, ab, n]}                        | ${'Nested objects match objects in collection'}
+        ${[a, { ...a, b: undefined }]}                                             | ${[a, a]}                                      | ${'Undefined values are removed.'}
+        ${[describe, undefined, null, 0, sym, sym]}                                | ${[describe, undefined, null, 0, sym, sym]}    | ${'random things'}
+        ${[arr, ['hello', a, b, c]]}                                               | ${[arr, arr]}                                  | ${'arrays'}
+        ${[cir, { values: [cir] }]}                                                | ${[cir, cir]}                                  | ${'circular'}
+        ${[nestedCircular, { cir }]}                                               | ${[nestedCircular, nestedCircular]}            | ${'nested circular'}
+        ${[a, f]}                                                                  | ${[a, a]}                                      | ${'first object wins'}
+        ${[f, a]}                                                                  | ${[f, f]}                                      | ${'reverse frozen objects'}
+    `('Collection $comment $objects', ({ objects, expected }: { objects: object[]; expected: unknown[] }) => {
+        const c = new Collection();
+        const r = objects.map((v) => c.add(v));
+        expect(r).toEqual(expected);
+        for (let i = 0; i < r.length; ++i) {
+            expect(r[i]).toBe(expected[i]);
+        }
+    });
+});