Block on jsonc errors as well as our own Azure errors when editing ta…

…gs (#61)
microsoft · Feb 18, 2021 · 8ff9f55 · 8ff9f55
1 parent b657a73
commit 8ff9f55
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 5 deletions.
diff --git a/src/commands/tags/TagFileSystem.ts b/src/commands/tags/TagFileSystem.ts
@@ -6,14 +6,13 @@
 import { ResourceManagementClient } from "@azure/arm-resources";
 import * as jsonc from 'jsonc-parser';
 import * as os from "os";
-import { commands, Diagnostic, FileStat, FileType, MessageItem, Uri, window } from "vscode";
+import { commands, Diagnostic, DiagnosticSeverity, FileStat, FileType, languages, MessageItem, Uri, window } from "vscode";
 import { AzExtTreeFileSystem, IActionContext } from 'vscode-azureextensionui';
 import { ext } from "../../extensionVariables";
 import { ResourceGroupTreeItem } from "../../tree/ResourceGroupTreeItem";
 import { ResourceTreeItem } from "../../tree/ResourceTreeItem";
 import { createResourceClient } from "../../utils/azureClients";
 import { localize } from "../../utils/localize";
-import { getTagDiagnostics } from "./getTagDiagnostics";
 
 const insertKeyHere: string = localize('insertTagName', '<Insert tag name>');
 const insertValueHere: string = localize('insertTagValue', '<Insert tag value>');
@@ -40,8 +39,7 @@ export class TagFileSystem extends AzExtTreeFileSystem<ResourceGroupTreeItem | R
         const text: string = content.toString();
         const isResourceGroup: boolean = node instanceof ResourceGroupTreeItem;
 
-        // tslint:disable-next-line: strict-boolean-expressions
-        const diagnostics: readonly Diagnostic[] = ext.diagnosticCollection.get(originalUri) || getTagDiagnostics(text);
+        const diagnostics: Diagnostic[] = languages.getDiagnostics(originalUri).filter(d => d.severity === DiagnosticSeverity.Error);
         if (diagnostics.length > 0) {
             context.telemetry.measurements.tagDiagnosticsLength = diagnostics.length;
 
@@ -61,6 +59,8 @@ export class TagFileSystem extends AzExtTreeFileSystem<ResourceGroupTreeItem | R
                 }
             });
 
+            // de-duped, sorted list of diagnostic sources
+            context.telemetry.properties.diagnosticSources = diagnostics.map(d => d.source).filter((value, index, array) => value && array.indexOf(value) === index).sort().join(',');
             context.errorHandling.suppressDisplay = true;
             // This won't be displayed, but might as well track the first diagnostic for telemetry
             throw new Error(diagnostics[0].message);

diff --git a/src/commands/tags/getTagDiagnostics.ts b/src/commands/tags/getTagDiagnostics.ts
@@ -27,6 +27,7 @@ class TagVisitor implements jsonc.JSONVisitor {
     private readonly _objectOpenBracketPositions: Position[] = [];
     private readonly _arrayOpenBracketPositions: Position[] = [];
     private _tagCount: number = 0;
+    private readonly _existingTags: string[] = [];
 
     /**
      * Invoked when an open brace is encountered and an object is started. The offset and length represent the location of the open brace.
@@ -94,6 +95,13 @@ class TagVisitor implements jsonc.JSONVisitor {
                 this.addError(range, error);
             }
 
+            if (this._existingTags.includes(property.toLowerCase())) {
+                const error: string = localize('tagNameAlreadyUsed', 'Tag name is already used. Tag names are case-insensitive.');
+                this.addError(range, error);
+            } else {
+                this._existingTags.push(property.toLowerCase());
+            }
+
             const maxTags: number = 50;
             if (this._tagCount > maxTags) {
                 this.addError(range, localize('tooManyTags', 'Only {0} tags are allowed.', maxTags));
@@ -128,6 +136,8 @@ class TagVisitor implements jsonc.JSONVisitor {
     }
 
     private addError(range: Range, error: string): void {
-        this.diagnostics.push(new Diagnostic(range, error));
+        const diagnostic: Diagnostic = new Diagnostic(range, error);
+        diagnostic.source = 'Azure';
+        this.diagnostics.push(diagnostic);
     }
 }