win, compiler: fix and validate comment-only code

This commit fixes scripts using `DeleteRegistryKey` having empty revert codes. It also adds validation in compiler to prevent genarating scripts composed solely of comments.
undergroundwires · Dec 18, 2024 · 2f2813e · 2f2813e
1 parent 92ec138
commit 2f2813e
Show file tree

Hide file tree

Showing 16 changed files with 422 additions and 35 deletions.
diff --git a/src/application/Parser/Executable/Script/Compiler/ScriptCompilerFactory.ts b/src/application/Parser/Executable/Script/Compiler/ScriptCompilerFactory.ts
@@ -80,7 +80,7 @@ class FunctionCallScriptCompiler implements ScriptCompiler {
       }
       const calls = parseFunctionCalls(script.call);
       const compiledCode = this.utilities.callCompiler.compileFunctionCalls(calls, this.functions);
-      validateCompiledCode(
+      validateFinalCompiledCode(
         compiledCode,
         this.language,
         this.utilities.codeValidator,
@@ -95,7 +95,7 @@ class FunctionCallScriptCompiler implements ScriptCompiler {
   }
 }
 
-function validateCompiledCode(
+function validateFinalCompiledCode(
   compiledCode: CompiledCode,
   language: ScriptingLanguage,
   validate: CodeValidator,
@@ -109,6 +109,7 @@ function validateCompiledCode(
           CodeValidationRule.NoEmptyLines,
           CodeValidationRule.NoTooLongLines,
           // Allow duplicated lines to enable calling same function multiple times
+          CodeValidationRule.NoCommentOnlyLines,
         ],
       ),
     );

diff --git a/src/application/Parser/Executable/Script/ScriptParser.ts b/src/application/Parser/Executable/Script/ScriptParser.ts
@@ -95,6 +95,7 @@ function validateHardcodedCodeWithoutCalls(
           CodeValidationRule.NoEmptyLines,
           CodeValidationRule.NoDuplicatedLines,
           CodeValidationRule.NoTooLongLines,
+          CodeValidationRule.NoCommentOnlyLines,
         ],
       ),
     );

diff --git a/src/application/Parser/Executable/Script/Validation/Analyzers/AnalyzeCommentOnlyCode.ts b/src/application/Parser/Executable/Script/Validation/Analyzers/AnalyzeCommentOnlyCode.ts
@@ -0,0 +1,33 @@
+import type { ScriptingLanguage } from '@/domain/ScriptingLanguage';
+import { isCommentLine, type CommentLineChecker } from './Common/CommentLineChecker';
+import { createSyntax, type SyntaxFactory } from './Syntax/SyntaxFactory';
+import type { CodeLine, CodeValidationAnalyzer, InvalidCodeLine } from './CodeValidationAnalyzer';
+
+export type CommentOnlyCodeAnalyzer = CodeValidationAnalyzer & {
+  (
+    ...args: [
+      ...Parameters<CodeValidationAnalyzer>,
+      syntaxFactory?: SyntaxFactory,
+      commentLineChecker?: CommentLineChecker,
+    ]
+  ): ReturnType<CodeValidationAnalyzer>;
+};
+
+export const analyzeCommentOnlyCode: CommentOnlyCodeAnalyzer = (
+  lines: readonly CodeLine[],
+  language: ScriptingLanguage,
+  syntaxFactory: SyntaxFactory = createSyntax,
+  commentLineChecker: CommentLineChecker = isCommentLine,
+) => {
+  const syntax = syntaxFactory(language);
+  if (!lines.every((line) => commentLineChecker(line.text, syntax))) {
+    return [];
+  }
+  return lines
+    .map((line): InvalidCodeLine => ({
+      lineNumber: line.lineNumber,
+      error: (() => {
+        return 'Code consists of comments only';
+      })(),
+    }));
+};
diff --git a/src/application/Parser/Executable/Script/Validation/Analyzers/AnalyzeDuplicateLines.ts b/src/application/Parser/Executable/Script/Validation/Analyzers/AnalyzeDuplicateLines.ts
@@ -1,13 +1,15 @@
 import type { LanguageSyntax } from '@/application/Parser/Executable/Script/Validation/Analyzers/Syntax/LanguageSyntax';
 import type { ScriptingLanguage } from '@/domain/ScriptingLanguage';
 import { createSyntax, type SyntaxFactory } from './Syntax/SyntaxFactory';
+import { isCommentLine, type CommentLineChecker } from './Common/CommentLineChecker';
 import type { CodeLine, CodeValidationAnalyzer, InvalidCodeLine } from './CodeValidationAnalyzer';
 
 export type DuplicateLinesAnalyzer = CodeValidationAnalyzer & {
   (
     ...args: [
       ...Parameters<CodeValidationAnalyzer>,
       syntaxFactory?: SyntaxFactory,
+      commentLineChecker?: CommentLineChecker,
     ]
   ): ReturnType<CodeValidationAnalyzer>;
 };
@@ -16,12 +18,13 @@ export const analyzeDuplicateLines: DuplicateLinesAnalyzer = (
   lines: readonly CodeLine[],
   language: ScriptingLanguage,
   syntaxFactory: SyntaxFactory = createSyntax,
+  commentLineChecker: CommentLineChecker = isCommentLine,
 ) => {
   const syntax = syntaxFactory(language);
   return lines
     .map((line): CodeLineWithDuplicateOccurrences => ({
       lineNumber: line.lineNumber,
-      shouldBeIgnoredInAnalysis: shouldIgnoreLine(line.text, syntax),
+      shouldBeIgnoredInAnalysis: shouldIgnoreLine(line.text, syntax, commentLineChecker),
       duplicateLineNumbers: lines
         .filter((other) => other.text === line.text)
         .map((duplicatedLine) => duplicatedLine.lineNumber),
@@ -43,17 +46,15 @@ function isNonIgnorableDuplicateLine(line: CodeLineWithDuplicateOccurrences): bo
   return !line.shouldBeIgnoredInAnalysis && line.duplicateLineNumbers.length > 1;
 }
 
-function shouldIgnoreLine(codeLine: string, syntax: LanguageSyntax): boolean {
-  return isCommentLine(codeLine, syntax)
+function shouldIgnoreLine(
+  codeLine: string,
+  syntax: LanguageSyntax,
+  commentLineChecker: CommentLineChecker,
+): boolean {
+  return commentLineChecker(codeLine, syntax)
     || isLineComposedEntirelyOfCommonCodeParts(codeLine, syntax);
 }
 
-function isCommentLine(codeLine: string, syntax: LanguageSyntax): boolean {
-  return syntax.commentDelimiters.some(
-    (delimiter) => codeLine.startsWith(delimiter),
-  );
-}
-
 function isLineComposedEntirelyOfCommonCodeParts(
   codeLine: string,
   syntax: LanguageSyntax,

diff --git a/src/application/Parser/Executable/Script/Validation/Analyzers/Common/CommentLineChecker.ts b/src/application/Parser/Executable/Script/Validation/Analyzers/Common/CommentLineChecker.ts
@@ -0,0 +1,14 @@
+import type { LanguageSyntax } from '../Syntax/LanguageSyntax';
+
+export interface CommentLineChecker {
+  (
+    codeLine: string,
+    syntax: LanguageSyntax,
+  ): boolean;
+}
+
+export const isCommentLine: CommentLineChecker = (codeLine, syntax) => {
+  return syntax.commentDelimiters.some(
+    (delimiter) => codeLine.toLowerCase().startsWith(delimiter.toLowerCase()),
+  );
+};
diff --git a/src/application/Parser/Executable/Script/Validation/CodeValidationRule.ts b/src/application/Parser/Executable/Script/Validation/CodeValidationRule.ts
@@ -2,4 +2,5 @@ export enum CodeValidationRule {
   NoEmptyLines,
   NoDuplicatedLines,
   NoTooLongLines,
+  NoCommentOnlyLines,
 }
diff --git a/src/application/Parser/Executable/Script/Validation/ValidationRuleAnalyzerFactory.ts b/src/application/Parser/Executable/Script/Validation/ValidationRuleAnalyzerFactory.ts
@@ -2,6 +2,7 @@ import { CodeValidationRule } from './CodeValidationRule';
 import { analyzeDuplicateLines } from './Analyzers/AnalyzeDuplicateLines';
 import { analyzeEmptyLines } from './Analyzers/AnalyzeEmptyLines';
 import { analyzeTooLongLines } from './Analyzers/AnalyzeTooLongLines';
+import { analyzeCommentOnlyCode } from './Analyzers/AnalyzeCommentOnlyCode';
 import type { CodeValidationAnalyzer } from './Analyzers/CodeValidationAnalyzer';
 
 export interface ValidationRuleAnalyzerFactory {
@@ -26,6 +27,8 @@ function createValidationRule(rule: CodeValidationRule): CodeValidationAnalyzer
       return analyzeDuplicateLines;
     case CodeValidationRule.NoTooLongLines:
       return analyzeTooLongLines;
+    case CodeValidationRule.NoCommentOnlyLines:
+      return analyzeCommentOnlyCode;
     default:
       throw new Error(`Unknown rule: ${rule}`);
   }

diff --git a/src/application/collections/windows.yaml b/src/application/collections/windows.yaml
@@ -40136,7 +40136,9 @@ functions:
                         Remove the registry key "{{ $keyPath }}"
                         {{ with $codeComment }}({{ . }}){{ end }}
                     revertCodeComment: >-
-                        Recreate the registry key "{{ $keyPath }}"
+                        {{ with $recreateOnRevert }}
+                            Recreate the registry key "{{ $keyPath }}"
+                        {{ end }}
             -
                 # Marked: refactor-with-variables
                 #   - Replacing SID is same as `CreateRegistryKey`

diff --git a/tests/unit/application/Parser/Executable/Script/Compiler/ScriptCompilerFactory.spec.ts b/tests/unit/application/Parser/Executable/Script/Compiler/ScriptCompilerFactory.spec.ts
@@ -234,6 +234,7 @@ describe('ScriptCompilerFactory', () => {
             CodeValidationRule.NoEmptyLines,
             CodeValidationRule.NoTooLongLines,
             // Allow duplicated lines to enable calling same function multiple times
+            CodeValidationRule.NoCommentOnlyLines,
           ];
           const scriptData = createScriptDataWithCall();
           const validator = new CodeValidatorStub();

diff --git a/tests/unit/application/Parser/Executable/Script/ScriptParser.spec.ts b/tests/unit/application/Parser/Executable/Script/ScriptParser.spec.ts
@@ -389,6 +389,7 @@ describe('ScriptParser', () => {
             CodeValidationRule.NoEmptyLines,
             CodeValidationRule.NoDuplicatedLines,
             CodeValidationRule.NoTooLongLines,
+            CodeValidationRule.NoCommentOnlyLines,
           ];
           const validator = new CodeValidatorStub();
           // act

diff --git a/.../application/Parser/Executable/Script/Validation/Analyzers/AnalyzeCommentOnlyCode.spec.ts b/.../application/Parser/Executable/Script/Validation/Analyzers/AnalyzeCommentOnlyCode.spec.ts
@@ -0,0 +1,133 @@
+import { describe, it } from 'vitest';
+import { analyzeCommentOnlyCode, type CommentOnlyCodeAnalyzer } from '@/application/Parser/Executable/Script/Validation/Analyzers/AnalyzeCommentOnlyCode';
+import type { CommentLineChecker } from '@/application/Parser/Executable/Script/Validation/Analyzers/Common/CommentLineChecker';
+import type { SyntaxFactory } from '@/application/Parser/Executable/Script/Validation/Analyzers/Syntax/SyntaxFactory';
+import { ScriptingLanguage } from '@/domain/ScriptingLanguage';
+import type { CodeLine, InvalidCodeLine } from '@/application/Parser/Executable/Script/Validation/Analyzers/CodeValidationAnalyzer';
+import { CommentLineCheckerStub } from '@tests/unit/shared/Stubs/CommentLineCheckerStub';
+import { SyntaxFactoryStub } from '@tests/unit/shared/Stubs/SyntaxFactoryStub';
+import { LanguageSyntaxStub } from '@tests/unit/shared/Stubs/LanguageSyntaxStub';
+import { createCodeLines } from './CreateCodeLines';
+import { expectSameInvalidCodeLines } from './ExpectSameInvalidCodeLines';
+
+describe('AnalyzeCommentOnlyCode', () => {
+  describe('analyzeCommentOnlyCode', () => {
+    it('returns empty given no match', () => {
+      // arrange
+      const context = setupScenario({
+        givenLines: ['line-1', 'line-2', 'line-3'],
+        matchedLines: [],
+      });
+      // act
+      const actualResult = context.analyze();
+      // assert
+      expect(actualResult).to.have.lengthOf(0);
+    });
+    it('returns empty given some matches', () => {
+      // arrange
+      const context = setupScenario({
+        givenLines: ['line-1', 'line-2'],
+        matchedLines: [],
+      });
+      // act
+      const actualResult = context.analyze();
+      // assert
+      expect(actualResult).to.have.lengthOf(0);
+    });
+    it('returns all lines given all match', () => {
+      // arrange
+      const lines = ['line-1', 'line-2', 'line-3'];
+      const expectedResult: InvalidCodeLine[] = lines
+        .map((_line, index): InvalidCodeLine => ({
+          lineNumber: index + 1,
+          error: 'Code consists of comments only',
+        }));
+      const context = setupScenario({
+        givenLines: lines,
+        matchedLines: lines,
+      });
+      // act
+      const actualResult = context.analyze();
+      // assert
+      expectSameInvalidCodeLines(expectedResult, actualResult);
+    });
+    it('uses correct language for syntax creation', () => {
+      // arrange
+      const expectedLanguage = ScriptingLanguage.batchfile;
+      let actualLanguage: ScriptingLanguage | undefined;
+      const factory: SyntaxFactory = (language) => {
+        actualLanguage = language;
+        return new LanguageSyntaxStub();
+      };
+      const context = new TestContext()
+        .withLanguage(expectedLanguage)
+        .withSyntaxFactory(factory);
+      // act
+      context.analyze();
+      // assert
+      expect(actualLanguage).to.equal(expectedLanguage);
+    });
+  });
+});
+
+interface CommentOnlyCodeAnalysisTestScenario {
+  readonly givenLines: readonly string[];
+  readonly matchedLines: readonly string[];
+}
+
+function setupScenario(
+  scenario: CommentOnlyCodeAnalysisTestScenario,
+): TestContext {
+  // arrange
+  const lines = scenario.givenLines;
+  const syntax = new LanguageSyntaxStub();
+  const checker = new CommentLineCheckerStub();
+  scenario.matchedLines.forEach((line) => checker.withPredeterminedResult({
+    givenLine: line,
+    givenSyntax: syntax,
+    result: true,
+  }));
+  return new TestContext()
+    .withSyntaxFactory(() => syntax)
+    .withLines(lines)
+    .withCommentLineChecker(checker.get());
+}
+
+export class TestContext {
+  private codeLines: readonly CodeLine[] = createCodeLines(['test-code-line']);
+
+  private language = ScriptingLanguage.batchfile;
+
+  private syntaxFactory: SyntaxFactory = new SyntaxFactoryStub().get();
+
+  private commentLineChecker: CommentLineChecker = new CommentLineCheckerStub().get();
+
+  public withLines(lines: readonly string[]): this {
+    this.codeLines = createCodeLines(lines);
+    return this;
+  }
+
+  public withLanguage(language: ScriptingLanguage): this {
+    this.language = language;
+    return this;
+  }
+
+  public withSyntaxFactory(syntaxFactory: SyntaxFactory): this {
+    this.syntaxFactory = syntaxFactory;
+    return this;
+  }
+
+  public withCommentLineChecker(commentLineChecker: CommentLineChecker): this {
+    this.commentLineChecker = commentLineChecker;
+    return this;
+  }
+
+  public analyze(): ReturnType<CommentOnlyCodeAnalyzer> {
+    return analyzeCommentOnlyCode(
+      this.codeLines,
+      this.language,
+      this.syntaxFactory,
+      this.commentLineChecker,
+    );
+  }
+}