Delete uninstalled imports (#1015)

* Handle optional dependencies

* Update documentation.

* Delete uninstalled package imports.

* Add more tests files to ensure skipped

Author: Chris McConnell

packages/dialog/README.md

@@ -36,7 +36,7 @@ _See code: [src/commands/dialog/index.ts](https://github.com/microsoft/botframew
 
 ## `bf dialog:merge PATTERNS`
 
-Merge `<kind>.schema` and `<kind>[.<locale>].uischema` definitions from a project and its dependencies into a single .schema for describing .dialog files and a per locale .uischema for describing how Composer shows them.  If a dependent package has an ExportedAssets directory it is copied to ImportedAssets/<package> in the --imports directory.
+Merge `<kind>.schema` and `<kind>[.<locale>].uischema` definitions from a project and its dependencies into a single .schema for describing .dialog files and a per locale .uischema for describing how Composer shows them.  If a dependent package has an "exported" directory it is copied to /<package> in the --imports directory.
 
 ```
 USAGE
@@ -48,12 +48,17 @@ ARGUMENTS
 OPTIONS
   -c, --checkOnly        Check and do not write files.
   -h, --help             show CLI help
-  -o, --output=output    Output path and filename for merged .schema and .uischema.  Defaults to first project name.
+
+  -o, --output=output    Output path and optional filename for merged .schema and .uischema.  Defaults to first project
+                         name.
+
   -s, --schema=schema    Path to merged .schema file to use if merging .uischema only.
+
   -v, --verbose          Show verbose logging of files as they are processed.
+
   --extension=extension  [default: .dialog,.lg,.lu,.schema,.qna,.uischema] Extension to include as a resource.
 
-  --imports=imports      Output path for imported assets.  Defaults to the directory of --out with an ImportedAssets
+  --imports=imports      Output path for imported assets.  Defaults to the directory of --out with an imported
                          directory.
 
 EXAMPLES

packages/dialog/src/library/schemaMerger.ts

@@ -157,7 +157,7 @@ class ComponentNode {
             patterns.push(`!${imports}`)
             patterns.push(`!${ppath.join(root, 'test/**')}`)
             patterns.push(`!${ppath.join(root, 'tests/**')}`)
-            patterns = patterns.concat(negativePatterns)
+            patterns = [...patterns, ...negativePatterns]
         }
         return patterns
     }
@@ -784,14 +784,16 @@ export class SchemaMerger {
             added: [], deleted: [], unchanged: [], conflicts: [],
             components: []
         }
-        if (imports && !this.schemaPath && this.components.length > 0) {
+        if (imports && !this.schemaPath) {
             this.log(`Copying exported assets to ${this.imports}`)
+            let processed = new Set<string>()
             for (let component of this.components) {
                 if (!component.isRoot()) {
                     let exported = ppath.join(ppath.dirname(component.metadata.path), 'exported')
                     if (component.isTopComponent()) {
                         imports.components.push(component.metadata)
                     }
+                    processed.add(component.metadata.name)
 
                     if (await fs.pathExists(exported)) {
                         let used = new Set<string>()
@@ -854,6 +856,27 @@ export class SchemaMerger {
                     }
                 }
             }
+
+            // Identify previously imported components that are not there any more
+            if (await fs.pathExists(this.imports)) {
+                for (let importedDir of await fs.readdir(this.imports)) {
+                    if (!processed.has(importedDir)) {
+                        for (let path of await glob(forwardSlashes(ppath.join(this.imports, importedDir, '/**')))) {
+                            let { unchanged } = await hash.isUnchanged(path)
+                            if (unchanged) {
+                                imports.deleted.push(path)
+                                this.vlog(`Delete ${path}`)
+                            } else {
+                                imports.conflicts.push({ definition: '', path })
+                                this.warn(`Warning deleted modified ${path}`)
+                            }
+                        }
+                        if (!this.checkOnly) {
+                            await fs.remove(importedDir)
+                        }
+                    }
+                }
+            }
         }
         return imports
     }
@@ -1197,20 +1220,26 @@ export class SchemaMerger {
                 this.packages.add(path)
                 this.vlog(`${this.indent()}Following ${this.prettyPath(path)}`)
                 let pkg = await fs.readJSON(path)
+                let dependencies = { ...pkg.dependencies, ...pkg.optionalDependencies }
                 this.pushParent(this.packageJsonComponent(path, pkg))
-                if (pkg.dependencies) {
-                    for (let dependent of Object.keys(pkg.dependencies)) {
+                if (dependencies) {
+                    for (let dependent of Object.keys(dependencies)) {
+                        let lastDir = ''
                         let rootDir = ppath.dirname(path)
                         // Walk up parent directories to find package
-                        while (rootDir) {
+                        while (rootDir !== lastDir) {
                             let dependentPath = ppath.join(rootDir, 'node_modules', dependent, 'package.json')
                             if (await fs.pathExists(dependentPath)) {
                                 await this.expandPackageJson(dependentPath)
                                 break
                             } else {
+                                lastDir = rootDir
                                 rootDir = ppath.dirname(rootDir)
                             }
                         }
+                        if (rootDir === lastDir && pkg.optionalDependencies && !(dependent in pkg.optionalDependencies)) {
+                            this.missingPackage(dependent)
+                        }
                     }
                 }
             } finally {
@@ -1598,7 +1627,7 @@ export class SchemaMerger {
                     if (this.definitions.hasOwnProperty(val.$kind)) {
                         val.$ref = '#/definitions/' + val.$kind
                     } else {
-                        this.missing(val.$kind)
+                        this.missingKind(val.$kind)
                     }
                 }
                 return false
@@ -1910,10 +1939,10 @@ export class SchemaMerger {
         }
     }
 
-    // Report missing component.
-    private missing(kind: string): void {
+    // Report missing $kind.
+    private missingKind(kind: string): void {
         if (!this.missingKinds.has(kind)) {
-            this.error(`${this.currentKind}: Missing ${kind} schema file from merge`)
+            this.error(`${this.currentKind}: Error missing ${kind} schema file from merge`)
             this.missingKinds.add(kind)
             this.failed = true
         }
@@ -1976,6 +2005,12 @@ export class SchemaMerger {
         this.failed = true
     }
 
+    // Report missing $kind.
+    private missingPackage(component: string): void {
+        this.error(`${this.currentFile}: Error could not find dependency ${component}`)
+        this.failed = true
+    }
+
     // Missing $ref
     private refError(original: string, modified: string): void {
         this.error(`Error could not bundle ${original} into ${modified}`)

packages/dialog/test/commands/dialog/merge.test.ts

@@ -127,7 +127,7 @@ function checkMerged(merged: merger.Imports | undefined, adds: number, conflicts
 }
 
 describe('dialog:merge', async () => {
-    before(async () => {
+    beforeEach(async () => {
         // If you want to regenerate the oracle *.schema files, run schemas/makeschemas.cmd
         await fs.remove(tempDir)
         await fs.mkdirp(tempDir)
@@ -345,6 +345,18 @@ describe('dialog:merge', async () => {
         assert((await fs.readFile(luPath, 'utf8')).includes('changed'), 'Did not write file')
         assert((await fs.readFile(jpgPath, 'utf8')).includes('changed'), 'Wrote file in check-only')
         assert(!await fs.pathExists(deletedPath), 'Did not delete file')
+
+        // Fifth import with component removed
+        console.log('\nFifth import removing reference')
+        await modifyFile(project, /<PackageReference.*/, '')
+        let [merged5, lines5] = await merge([project], 'project3.schema', true, undefined, false)
+        assert(merged5, 'Could not merge 5th')
+        assert(countMatches(/error/i, lines5) === 1, 'Error merging schemas 5th')
+        assert(countMatches(/warning/i, lines5) === 1, 'Wrong number of warnings 5th')
+        assert(merged5?.added.length === 0, 'Wrong number added 5th')
+        assert(merged5?.deleted.length === 4, 'Wrong number deleted 5th')
+        assert(merged5?.unchanged.length === 0, 'Wrong number unchanged 5th')
+        assert(merged5?.conflicts.length === 1, 'Wrong number of conflicts on 5th')
     })
 
     it('package.json', async () => {

packages/dialog/test/commands/dialog/nuget/nuget3/1.0.0/tests/nuget3.lu

@@ -0,0 +1,2 @@
+# intent
+- intent