support else directive. #185 AG-26383

Squashed commit of the following: commit 1e6e92b Author: Slava Leleka <[email protected]> Date: Tue Nov 7 17:58:50 2023 +0200 fix notes style commit a936cd7 Author: Slava Leleka <[email protected]> Date: Tue Nov 7 17:52:47 2023 +0200 fix note style commit 1a47a6d Author: Slava Leleka <[email protected]> Date: Tue Nov 7 17:49:00 2023 +0200 fix description for unknown-preprocessor-directives commit f4429ac Author: Slava Leleka <[email protected]> Date: Tue Nov 7 17:35:54 2023 +0200 update readme commit d942577 Author: Slava Leleka <[email protected]> Date: Tue Nov 7 17:17:48 2023 +0200 improve tests and IfClosed() commit 87c5d76 Author: Slava Leleka <[email protected]> Date: Tue Nov 7 16:09:57 2023 +0200 add more tests commit 131cf46 Author: Slava Leleka <[email protected]> Date: Tue Nov 7 16:05:55 2023 +0200 fix alphabetical order commit 54fff10 Author: Slava Leleka <[email protected]> Date: Tue Nov 7 16:04:56 2023 +0200 refactor IfClosed() commit 73deab3 Author: Slava Leleka <[email protected]> Date: Tue Nov 7 14:58:37 2023 +0200 fix if nesting commit 5c5a9d6 Author: Slava Leleka <[email protected]> Date: Tue Nov 7 14:31:02 2023 +0200 support else directive
AdguardTeam · Nov 7, 2023 · 24f7c36 · 24f7c36
1 parent b47bfdf
commit 24f7c36
Show file tree

Hide file tree

Showing 11 changed files with 383 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,19 +5,26 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog][keepachangelog], and this project adheres to [Semantic Versioning][semver].
 
 
+## [Unreleased]
+
+### Added
+
+- Support of `!#else` preprocessor directive [#185].
+
+
 ## [2.0.5] - 2023-09-07
 
 ### Changed
 
-- Updated `@adguard/agtree` to `v1.1.5`.
+- Updated [@adguard/agtree] to `v1.1.5`.
 
 
 ## [2.0.4] - 2023-08-30
 
 ### Changed
 
 - Override `config.extends`'s presets by the user config.
-- Updated `@adguard/agtree` to `v1.1.4`.
+- Updated [@adguard/agtree] to `v1.1.4`.
 
 
 ## [2.0.3] - 2023-08-29
@@ -30,15 +37,15 @@ The format is based on [Keep a Changelog][keepachangelog], and this project adhe
 
 ### Changed
 
-- Updated `@adguard/agtree` to `v1.1.3`.
+- Updated [@adguard/agtree] to `v1.1.3`.
 
 
 ## [2.0.1] - 2023-08-14
 
 ### Changed
 
 - Make `syntax` property in the config required.
-- Updated `@adguard/agtree` to `v1.1.2`.
+- Updated [@adguard/agtree] to `v1.1.2`.
 
 
 ## [2.0.0] - 2023-08-11
@@ -50,7 +57,7 @@ The format is based on [Keep a Changelog][keepachangelog], and this project adhe
 
 ### Changed
 
-- Updated `@adguard/agtree` to `v1.1.1`.
+- Updated [@adguard/agtree] to `v1.1.1`.
 
 
 ## [1.0.11] - 2023-04-21
@@ -139,6 +146,7 @@ The format is based on [Keep a Changelog][keepachangelog], and this project adhe
 - Initial version of the linter and CLI.
 - Initial version of the adblock rule parser.
 
+[Unreleased]: https://github.com/AdguardTeam/AGLint/compare/v2.0.5...HEAD
 [2.0.5]: https://github.com/AdguardTeam/AGLint/compare/v2.0.4...v2.0.5
 [2.0.4]: https://github.com/AdguardTeam/AGLint/compare/v2.0.3...v2.0.4
 [2.0.3]: https://github.com/AdguardTeam/AGLint/compare/v2.0.1...v2.0.3
@@ -155,4 +163,7 @@ The format is based on [Keep a Changelog][keepachangelog], and this project adhe
 
 [keepachangelog]: https://keepachangelog.com/en/1.0.0/
 [semver]: https://semver.org/spec/v2.0.0.html
+[@adguard/agtree]: https://github.com/AdguardTeam/tsurlfilter/blob/master/packages/agtree/CHANGELOG.md
+
 [#10]: https://github.com/AdguardTeam/AGLint/issues/10
+[#185]: https://github.com/AdguardTeam/AGLint/issues/185
diff --git a/README.md b/README.md
@@ -125,13 +125,16 @@ it as a CLI tool with the default configuration.
 
 That's all! :hugs: The linter will check all filter lists in your project and print the results to the console.
 
-> **Note**: You can also install AGLint globally, so you can use it without `npx` or `yarn`, but we recommend to install
+> [!NOTE]
+> You can also install AGLint globally, so you can use it without `npx` or `yarn`, but we recommend to install
 > it locally to your project.
 
-> **Note**:  If you want to lint just some specific files, you can pass them as arguments:
+> [!NOTE]
+> If you want to lint just some specific files, you can pass them as arguments:
 > `aglint path/to/file.txt path/to/another/file.txt`
 
-> **Note**: To see all available options, run `aglint --help`.
+> [!NOTE]
+> To see all available options, run `aglint --help`.
 
 *To customize the default configuration, see [Configuration](#configuration) for more info. If you want to use AGLint
 programmatically, see [Use programmatically](#use-programmatically).*
@@ -264,7 +267,8 @@ project.** This command will create a `.aglintrc.yaml` file in the current direc
 
 You can also create a configuration file manually, please check the section below for more info.
 
-> **Note**: We are planning to add a configuration wizard in the future, so you will be able to create a configuration
+> [!NOTE]
+> We are planning to add a configuration wizard in the future, so you will be able to create a configuration
 > file by answering a few questions.
 
 ### Configuration file name and format
@@ -282,13 +286,16 @@ We also plan to support `.aglintrc.js` (JavaScript) in the future.
 We recommend using `.aglintrc.yaml` or `.aglintrc.yml` because YAML is more compact and easier to read, and it supports
 comments.
 
-> **Warning**: If you have multiple configuration files in the same directory, the CLI will throw an error and ask you
+> [!WARNING]
+> If you have multiple configuration files in the same directory, the CLI will throw an error and ask you
 > to fix it.
 
-> **Warning**: If your configuration file is syntactically invalid or contains unknown / invalid options, the CLI will
+> [!WARNING]
+> If your configuration file is syntactically invalid or contains unknown / invalid options, the CLI will
 > throw an error and ask you to fix it.
 
-> **Warning**: If your configuration file is not named in one of the ways listed above, the CLI will ignore it (since it
+> [!WARNING]
+> If your configuration file is not named in one of the ways listed above, the CLI will ignore it (since it
 > cannot recognize it as a configuration file).
 
 ### Configuration file structure
@@ -389,13 +396,16 @@ Currently, there are two built-in presets available (click on the name to see th
 - [`aglint:all`][aglint-all] — a set of **all** rules that are available in the linter.
   This option maybe too strict for most projects.
 
-> **Note**: Presets contain `syntax` and `rules` which shall be overridden if they are specified in the config.
+> [!NOTE]
+> Presets contain `syntax` and `rules` which shall be overridden if they are specified in the config.
 
-> **Note**: All presets have `syntax` property set to `Common` a default value.
+> [!NOTE]
+> All presets have `syntax` property set to `Common` a default value.
 > You may need to specify it in your [configuration file](#configuration-file-structure)
 > for better linting, e.g. modifiers validation.
 
-> **Note**: We are planning to add more presets in the future,
+> [!NOTE]
+> We are planning to add more presets in the future,
 > and also allow users to create their own presets but currently it is not possible.
 
 ### Default configuration file
@@ -428,7 +438,8 @@ It simply extends the `aglint:recommended` preset and specifies the `root` optio
     }
     ```
 
-> **Note**: JavaScript configuration files aren't supported at the moment
+> [!NOTE]
+> JavaScript configuration files aren't supported at the moment
 > but we plan to add support for them in the future (CJS and ESM syntaxes).
 
 ### Configuration cascading and hierarchy
@@ -522,7 +533,9 @@ Currently, the following linter rules are available (we will add more rules in t
 
 ### `if-closed`
 
-Checks if the `if` statement is closed and no unclosed `endif` statements are present.
+Checks if the `if` statement is closed and no unclosed `endif` or unopened `else` statements are present.
+It also checks whether `else` and `endif` statements are used correctly
+since they can only be used alone without other parameters or statements.
 
 - **Severity:** `error` (2)
 - **Options:** none
@@ -535,11 +548,14 @@ Checks if the `if` statement is closed and no unclosed `endif` statements are pr
   !#endif
   !#if (adguard_ext_firefox)
   example.org##.something
+  !#else if (adguard_ext_opera)
+  example.org##.operaBanner
   ```
   will be reported as error:
   ```txt
     1:0  error  Using an "endif" directive without an opening "if" directive
     5:0  error  Unclosed "if" directive
+    7:0  error  Invalid usage of preprocessor directive: "else"
   ```
   since the first `endif` are unnecessary, and the last `if` statement is not closed.
 
@@ -591,6 +607,9 @@ Checks if the same modifier is used multiple times in a single network rule.
 
 Checks if the used preprocessor directives are known.
 
+> [!IMPORTANT]
+> Preprocessor directives are case-sensitive, so `!#IF` is to be considered as invalid.
+
 - **Severity:** `error` (2)
 - **Options:** none
 - **Fixable:** no
@@ -606,6 +625,7 @@ Checks if the used preprocessor directives are known.
 - **Additional information:**
     - Currently, the following preprocessor directives are supported:
         - `if`: [reference][if-kb]
+        - `else`: [reference][if-kb]
         - `endif`: [reference][if-kb]
         - `include`: [reference][include-kb]
         - `safari_cb_affinity`: [reference][safari-cb-kb]

diff --git a/docs/repo-integration.md b/docs/repo-integration.md
@@ -208,7 +208,8 @@ AGLint by adding `--no-verify` to your commit command.
 Next time you clone the repository, you'll only need to run `npm install` and it will install Husky automatically,
 because it automatically runs `prepare` script after installing dependencies.
 
-> **Note**: To make the hook work in Unix-like operating systems you may need to run
+> [!NOTE]
+> To make the hook work in Unix-like operating systems you may need to run
 > `chmod ug+x .husky/pre-commit`.
 
 ### <a name="post-merge"></a> Add post-merge hook
@@ -217,7 +218,8 @@ If AGLint is updated on the remote repository (in practice, `package.json` or `p
 you need to update AGLint version in your local repository. If you just clone the changes, that does not sync your
 dependencies in `node_modules` directory, so you need to run `npm install` to sync your dependencies manually.
 
-> :warning: **Warning**: If you do not sync your installed dependencies in your `node_modules` folder
+> [!WARNING]
+> If you do not sync your installed dependencies in your `node_modules` folder
 > after the `package.json` or `package-lock.json` is changed,
 > you may get errors when running AGLint or your CI may report different results than your local machine.
 
@@ -258,7 +260,8 @@ fi
 This script will check if `package.json` or `package-lock.json` is changed between the old HEAD and the new HEAD.
 If one of them is changed, it will run `npm install` to sync your dependencies, so you don't need to do that manually.
 
-> **Note**: To make the hook work in Unix-like operating systems you may need to run
+> [!NOTE]
+> To make the hook work in Unix-like operating systems you may need to run
 > `chmod ug+x .husky/post-merge`.
 
 

diff --git a/src/common/constants.ts b/src/common/constants.ts
@@ -204,6 +204,16 @@ export const PREPROCESSOR_MARKER = '!#';
 export const PREPROCESSOR_MARKER_LEN = PREPROCESSOR_MARKER.length;
 export const PREPROCESSOR_SEPARATOR = ' ';
 
-export const SAFARI_CB_AFFINITY = 'safari_cb_affinity';
-export const IF = 'if';
-export const INCLUDE = 'include';
+export const IF_DIRECTIVE = 'if';
+export const ELSE_DIRECTIVE = 'else';
+export const ENDIF_DIRECTIVE = 'endif';
+export const INCLUDE_DIRECTIVE = 'include';
+export const SAFARI_CB_AFFINITY_DIRECTIVE = 'safari_cb_affinity';
+
+export const SUPPORTED_PREPROCESSOR_DIRECTIVES = new Set([
+    ELSE_DIRECTIVE,
+    ENDIF_DIRECTIVE,
+    IF_DIRECTIVE,
+    INCLUDE_DIRECTIVE,
+    SAFARI_CB_AFFINITY_DIRECTIVE,
+]);
diff --git a/src/linter/rules/README.md b/src/linter/rules/README.md
@@ -167,7 +167,8 @@ export const ExampleRule: LinterRule<LinterRuleStorage<unknown>, RuleConfig> = {
 };
 ```
 
-> **Note**: You can use both the rule storage and the parameters at the same time.
+> [!NOTE]
+> You can use both the rule storage and the parameters at the same time.
 
 ### Report problems
 
@@ -275,7 +276,8 @@ export const RuleName: LinterRule = {
 };
 ```
 
-> **Note**: If multiple fixes are suggested for the same problem, then the linter will ignore all of them in order to
+> [!NOTE]
+> If multiple fixes are suggested for the same problem, then the linter will ignore all of them in order to
 > avoid conflicts.
 
 [main-readme-linter-rules]: ../../../README.md#linter-rules

diff --git a/src/linter/rules/if-closed.ts b/src/linter/rules/if-closed.ts
@@ -2,9 +2,7 @@ import { CommentRuleType, type PreProcessorCommentRule, RuleCategory } from '@ad
 
 import { type LinterRule } from '../common';
 import { SEVERITY } from '../severity';
-
-const IF_DIRECTIVE = 'if';
-const ENDIF_DIRECTIVE = 'endif';
+import { ELSE_DIRECTIVE, ENDIF_DIRECTIVE, IF_DIRECTIVE } from '../../common/constants';
 
 /**
  * Concreting the storage type definition (the linter only provides a general
@@ -35,12 +33,43 @@ export const IfClosed: LinterRule<Storage> = {
             const rule = context.getActualAdblockRuleAst();
 
             // Check adblock rule category and type
-            if (rule.category === RuleCategory.Comment && rule.type === CommentRuleType.PreProcessorCommentRule) {
-                // Check for "if" and "endif" directives
-                if (rule.name.value === IF_DIRECTIVE) {
+            if (rule.category !== RuleCategory.Comment
+                || rule.type !== CommentRuleType.PreProcessorCommentRule) {
+                return;
+            }
+
+            const directive = rule.name.value;
+            switch (directive) {
+                case IF_DIRECTIVE:
                     // Collect open "if"
                     context.storage.openIfs.push(rule);
-                } else if (rule.name.value === ENDIF_DIRECTIVE) {
+                    break;
+                case ELSE_DIRECTIVE:
+                    // '!#else' can only be used alone without any parameters
+                    if (rule.params) {
+                        context.report({
+                            message: `Invalid usage of preprocessor directive: "${ELSE_DIRECTIVE}"`,
+                            node: rule,
+                        });
+                    }
+                    // Check if there is an open "!#if" before "!#else"
+                    if (context.storage.openIfs.length === 0) {
+                        context.report({
+                            // eslint-disable-next-line max-len
+                            message: `Using an "${ELSE_DIRECTIVE}" directive without an opening "${IF_DIRECTIVE}" directive`,
+                            node: rule,
+                        });
+                    }
+                    // otherwise do nothing
+                    break;
+                case ENDIF_DIRECTIVE:
+                    // '!#endif' can only be used alone without any parameters
+                    if (rule.params) {
+                        context.report({
+                            message: `Invalid usage of preprocessor directive: "${ENDIF_DIRECTIVE}"`,
+                            node: rule,
+                        });
+                    }
                     if (context.storage.openIfs.length === 0) {
                         context.report({
                             // eslint-disable-next-line max-len
@@ -51,7 +80,11 @@ export const IfClosed: LinterRule<Storage> = {
                         // Mark "if" as closed (simply delete it from collection)
                         context.storage.openIfs.pop();
                     }
-                }
+                    break;
+                default:
+                    // unsupported directives should be reported by another rule - 'unknown-preprocessor-directives'
+                    // so do nothing
+                    break;
             }
         },
         onEndFilterList: (context): void => {

diff --git a/src/linter/rules/unknown-preprocessor-directives.ts b/src/linter/rules/unknown-preprocessor-directives.ts
@@ -2,13 +2,7 @@ import { CommentRuleType, RuleCategory } from '@adguard/agtree';
 
 import { SEVERITY } from '../severity';
 import { type LinterRule } from '../common';
-
-const COMMON_PREPROCESSOR_DIRECTIVES = new Set([
-    'if',
-    'endif',
-    'include',
-    'safari_cb_affinity',
-]);
+import { SUPPORTED_PREPROCESSOR_DIRECTIVES } from '../../common/constants';
 
 /**
  * Checks if a preprocessor directive is known
@@ -17,11 +11,13 @@ const COMMON_PREPROCESSOR_DIRECTIVES = new Set([
  * @returns `true` if the preprocessor directive is known, `false` otherwise
  */
 function isKnownPreProcessorDirective(name: string): boolean {
-    return COMMON_PREPROCESSOR_DIRECTIVES.has(name);
+    return SUPPORTED_PREPROCESSOR_DIRECTIVES.has(name);
 }
 
 /**
- * Rule that checks if a preprocessor directive is known
+ * Rule that checks if a preprocessor directive is known.
+ *
+ * Directives are case-sensitive, so `!#IF` is to be considered as invalid.
  */
 export const UnknownPreProcessorDirectives: LinterRule = {
     meta: {