Merge #3770 from aws/samscan

Author: justinmk3

.changes/next-release/Feature-f2e37981-fba6-4a29-bdf3-4e06fa4dfa2f.json

@@ -0,0 +1,4 @@
+{
+	"type": "Feature",
+	"description": "SAM template detection now skips directories specified in [user settings](https://code.visualstudio.com/docs/getstarted/settings) `files.exclude`, `search.exclude`, or `files.watcherExclude`. This improves performance on big workspaces and avoids the \"Scanning CloudFormation templates...\" message. [#3510](https://github.com/aws/aws-toolkit-vscode/issues/3510)"
+}

src/shared/fs/watchedFiles.ts

@@ -7,14 +7,27 @@ import * as vscode from 'vscode'
 import { getLogger } from '../logger/logger'
 import * as pathutils from '../utilities/pathUtils'
 import * as path from 'path'
-import { isUntitledScheme, normalizeVSCodeUri } from '../utilities/vsCodeUtils'
+import { globDirs, isUntitledScheme, normalizeVSCodeUri } from '../utilities/vsCodeUtils'
+import { Settings } from '../settings'
+import { once } from '../utilities/functionUtils'
 
 /**
  * Prevent `findFiles()` from recursing into these directories.
  *
  * `findFiles()` defaults to the vscode `files.exclude` setting, which by default does not exclude "node_modules/".
  */
-const alwaysExclude = '**/{.aws-sam,.git,.svn,.hg,.rvm,.gem,.project,node_modules,venv,bower_components}/'
+const alwaysExclude = {
+    '.aws-sam': true,
+    '.git': true,
+    '.svn': true,
+    '.hg': true,
+    '.rvm': true,
+    '.gem': true,
+    '.project': true,
+    node_modules: true,
+    venv: true,
+    bower_components: true,
+}
 
 export interface WatchedItem<T> {
     /**
@@ -27,6 +40,21 @@ export interface WatchedItem<T> {
     item: T
 }
 
+/** Builds an exclude pattern based on vscode global settings and the `alwaysExclude` default. */
+export function getExcludePattern() {
+    const vscodeFilesExclude = Settings.instance.get<object>('files.exclude', Object, {})
+    const vscodeSearchExclude = Settings.instance.get<object>('search.exclude', Object, {})
+    const vscodeWatcherExclude = Settings.instance.get<object>('files.watcherExclude', Object, {})
+    const all = [
+        ...Object.keys(alwaysExclude),
+        ...Object.keys(vscodeFilesExclude),
+        ...Object.keys(vscodeSearchExclude),
+        ...Object.keys(vscodeWatcherExclude),
+    ]
+    return globDirs(all)
+}
+const getExcludePatternOnce = once(getExcludePattern)
+
 /**
  * WatchedFiles lets us index files in the current registry. It is used
  * for CFN templates among other things. WatchedFiles holds a list of pairs of
@@ -230,10 +258,19 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
      */
     public async rebuild(): Promise<void> {
         this.reset()
+
+        const exclude = getExcludePatternOnce()
         for (const glob of this.globs) {
-            const itemUris = await vscode.workspace.findFiles(glob, alwaysExclude)
-            for (const item of itemUris) {
-                await this.addItemToRegistry(item, true)
+            try {
+                const found = await vscode.workspace.findFiles(glob, exclude)
+                for (const item of found) {
+                    await this.addItemToRegistry(item, true)
+                }
+            } catch (e) {
+                const err = e as Error
+                if (err.name !== 'Canceled') {
+                    getLogger().error('watchedFiles: findFiles("%s", "%s"): %s', glob, exclude, err.message)
+                }
             }
         }
     }

src/shared/logger/activation.ts

@@ -48,7 +48,7 @@ export async function activate(
     )
 
     setLogger(mainLogger)
-    getLogger().error(`log level: ${getLogLevel()}`)
+    getLogger().info(`log level: ${getLogLevel()}`)
 
     // channel logger
     setLogger(

src/shared/settings.ts

@@ -31,19 +31,35 @@ export class Settings {
     ) {}
 
     /**
-     * Reads a setting, applying the {@link TypeConstructor} if provided.
+     * Gets a setting value, applying the {@link TypeConstructor} if provided.
      *
-     * Note that the absence of a key is indistinguisable from the absence of a value.
-     * That is, `undefined` looks the same across all calls. Any non-existent values are
-     * simply returned as-is without passing through the type cast.
+     * If the read fails or the setting value is invalid (does not conform to `type`):
+     * - `defaultValue` is returned if it was provided
+     * - else an exception is thrown
+     *
+     * @note An unknown setting is indistinguisable from a missing value: both return `undefined`.
+     * Non-existent values are returned as-is without passing through the type cast.
+     *
+     * @param key Setting name
+     * @param type Expected setting type
+     * @param defaultValue Value returned if setting is missing or invalid
      */
     public get(key: string): unknown
     public get<T>(key: string, type: TypeConstructor<T>): T | undefined
     public get<T>(key: string, type: TypeConstructor<T>, defaultValue: T): T
     public get<T>(key: string, type?: TypeConstructor<T>, defaultValue?: T) {
-        const value = this.getConfig().get(key, defaultValue)
+        try {
+            const value = this.getConfig().get(key, defaultValue)
 
-        return !type || value === undefined ? value : cast(value, type)
+            return !type || value === undefined ? value : cast(value, type)
+        } catch (e) {
+            if (arguments.length <= 2) {
+                throw ToolkitError.chain(e, `Failed to read setting "${key}"`)
+            }
+            getLogger().error('settings: failed to read "%s": %s', key, (e as Error).message)
+
+            return defaultValue
+        }
     }
 
     /**
@@ -62,8 +78,8 @@ export class Settings {
             await this.getConfig().update(key, value, this.updateTarget)
 
             return true
-        } catch (error) {
-            getLogger().warn(`Settings: failed to update "${key}": %s`, error)
+        } catch (e) {
+            getLogger().warn('settings: failed to update "%s": %s', key, (e as Error).message)
 
             return false
         }
@@ -151,7 +167,8 @@ export class Settings {
      * The resulting configuration object should not be cached.
      */
     private getConfig(section?: string) {
-        return this.workspace.getConfiguration(section, this.scope)
+        // eslint-disable-next-line no-null/no-null
+        return this.workspace.getConfiguration(section, this.scope ?? null)
     }
 
     /**
@@ -167,7 +184,7 @@ export class Settings {
     static #instance: Settings
 
     /**
-     * A singleton scoped to the global configuration target.
+     * A singleton scoped to the global configuration target and `null` resource.
      */
     public static get instance() {
         return (this.#instance ??= new this())
@@ -223,15 +240,24 @@ function createSettingsClass<T extends TypeDescriptor>(section: string, descript
             return this.config.keys()
         }
 
+        /**
+         * Gets a setting value.
+         *
+         * If the read fails or the setting value is invalid (does not conform to the type defined in package.json):
+         * - `defaultValue` is returned if it was provided
+         * - else an exception is thrown
+         *
+         * @param key Setting name
+         * @param defaultValue Value returned if setting is missing or invalid
+         */
         public get<K extends keyof Inner>(key: K & string, defaultValue?: Inner[K]) {
             try {
                 return this.getOrThrow(key, defaultValue)
             } catch (e) {
                 if (arguments.length === 1) {
                     throw ToolkitError.chain(e, `Failed to read key "${section}.${key}"`)
                 }
-
-                this.logErr(`using default for key "${key}", read failed: %s`, e)
+                this.logErr('failed to read "%s": %s', key, (e as Error).message)
 
                 return defaultValue as Inner[K]
             }
@@ -242,8 +268,8 @@ function createSettingsClass<T extends TypeDescriptor>(section: string, descript
                 await this.config.update(key, value)
 
                 return true
-            } catch (error) {
-                this.log(`failed to update field "${key}": %s`, error)
+            } catch (e) {
+                this.log('failed to update "%s": %s', key, (e as Error).message)
 
                 return false
             }
@@ -254,8 +280,8 @@ function createSettingsClass<T extends TypeDescriptor>(section: string, descript
                 await this.config.update(key, undefined)
 
                 return true
-            } catch (error) {
-                this.log(`failed to delete field "${key}": %s`, error)
+            } catch (e) {
+                this.log('failed to delete "%s": %s', key, (e as Error).message)
 
                 return false
             }
@@ -264,8 +290,8 @@ function createSettingsClass<T extends TypeDescriptor>(section: string, descript
         public async reset() {
             try {
                 return await this.config.reset()
-            } catch (error) {
-                this.log(`failed to reset settings: %s`, error)
+            } catch (e) {
+                this.log('failed to reset settings: %s', (e as Error).message)
             }
         }
 
@@ -316,7 +342,7 @@ function createSettingsClass<T extends TypeDescriptor>(section: string, descript
                 }
 
                 for (const key of props.filter(isDifferent)) {
-                    this.log(`key "${key}" changed`)
+                    this.log('key "%s" changed', key)
                     emitter.fire({ key })
                 }
             })
@@ -508,8 +534,8 @@ export class PromptSettings extends Settings.define(
     public async isPromptEnabled(promptName: PromptName): Promise<boolean> {
         try {
             return !this.getOrThrow(promptName, false)
-        } catch (error) {
-            this.log(`prompt check for "${promptName}" failed: %s`, error)
+        } catch (e) {
+            this.log('prompt check for "%s" failed: %s', promptName, (e as Error).message)
             await this.reset()
 
             return true

src/shared/utilities/vsCodeUtils.ts

@@ -169,6 +169,30 @@ export function isUntitledScheme(uri: vscode.Uri): boolean {
     return uri.scheme === 'untitled'
 }
 
+/**
+ * Creates a glob pattern that matches all directories specified in `dirs`.
+ *
+ * "/" and "*" chars are trimmed from `dirs` items, and the final glob is defined such that the
+ * directories and their contents are matched any _any_ depth.
+ *
+ * Example: `['foo', '**\/bar/'] => "**\/{foo,bar}/"`
+ */
+export function globDirs(dirs: string[]): string {
+    const excludePatternsStr = dirs.reduce((prev, current) => {
+        // Trim all "*" and "/" chars.
+        // Note that the replace() patterns and order is intentionaly so that "**/*foo*/**" yields "*foo*".
+        const scrubbed = current
+            .replace(/^\**/, '')
+            .replace(/^[/\\]*/, '')
+            .replace(/\**$/, '')
+            .replace(/[/\\]*$/, '')
+        const comma = prev === '' ? '' : ','
+        return `${prev}${comma}${scrubbed}`
+    }, '')
+    const excludePattern = `**/{${excludePatternsStr}}/`
+    return excludePattern
+}
+
 // If the VSCode URI is not a file then return the string representation, otherwise normalize the filesystem path
 export function normalizeVSCodeUri(uri: vscode.Uri): string {
     if (uri.scheme !== 'file') {

src/test/shared/settingsConfiguration.test.ts

@@ -72,6 +72,10 @@ describe('Settings', function () {
             assert.throws(() => sut.get(settingKey, String))
             assert.throws(() => sut.get(settingKey, Object))
             assert.throws(() => sut.get(settingKey, Boolean))
+            // Wrong type, but defaultValue was given:
+            assert.deepStrictEqual(sut.get(settingKey, String, ''), '')
+            assert.deepStrictEqual(sut.get(settingKey, Object, {}), {})
+            assert.deepStrictEqual(sut.get(settingKey, Boolean, true), true)
         })
     })
 

src/test/shared/utilities/vscodeUtils.test.ts

@@ -7,6 +7,7 @@ import * as assert from 'assert'
 import { VSCODE_EXTENSION_ID } from '../../../shared/extensions'
 import * as vscodeUtil from '../../../shared/utilities/vsCodeUtils'
 import * as vscode from 'vscode'
+import { getExcludePattern } from '../../../shared/fs/watchedFiles'
 
 describe('vscodeUtils', async function () {
     it('activateExtension(), isExtensionActive()', async function () {
@@ -20,6 +21,17 @@ describe('vscodeUtils', async function () {
         await vscodeUtil.activateExtension(VSCODE_EXTENSION_ID.awstoolkit, false)
         assert.deepStrictEqual(vscodeUtil.isExtensionActive(VSCODE_EXTENSION_ID.awstoolkit), true)
     })
+
+    it('globDirs()', async function () {
+        const input = ['foo', '**/bar/**', '*baz*', '**/*with.star*/**', '/zub', 'zim/', '/zoo/']
+        assert.deepStrictEqual(vscodeUtil.globDirs(input), '**/{foo,bar,baz,*with.star*,zub,zim,zoo}/')
+    })
+
+    it('watchedFiles.getExcludePattern()', async function () {
+        // If vscode defaults change in the future, just update this test.
+        // We intentionally want visibility into real-world defaults.
+        assert.match(getExcludePattern(), /node_modules,bower_components,\*\.code-search,/)
+    })
 })
 
 describe('isExtensionInstalled()', function () {