build: enforce that we use comments that won't show up in the generated output (angular#22309)

crisbeto · web-flow · commit 3c975e9304d6 · 2021-03-23T09:35:48.000-06:00
Adds a custom stylelint rule that will prevent multi-line comments (`/* */`) from being used in .scss files. The problem with such comments are preserved by Sass and will show up in the user's output, whereas single-line comments (`//`) will not.

**Note:** while Stylelint has a `comment-pattern` rule already, we can't use it because the rule works on the comment text which doesn't include the syntax.
diff --git a/.stylelintrc.json b/.stylelintrc.json
@@ -9,7 +9,8 @@
     "./tools/stylelint/no-concrete-rules.ts",
     "./tools/stylelint/no-top-level-ampersand-in-mixin.ts",
     "./tools/stylelint/theme-mixin-api.ts",
-    "./tools/stylelint/no-import.ts"
+    "./tools/stylelint/no-import.ts",
+    "./tools/stylelint/single-line-comment-only.ts"
   ],
   "rules": {
     "material/no-prefixes": [true, {
@@ -19,6 +20,9 @@
     "material/theme-mixin-api": true,
     "material/selector-no-deep": true,
     "material/no-nested-mixin": true,
+    "material/single-line-comment-only": [true, {
+      "filePattern": "\\.scss$"
+    }],
     "material/no-import": [true, {
       "exclude": "\\.import\\.scss$"
     }],
diff --git a/src/dev-app/virtual-scroll/virtual-scroll-demo.scss b/src/dev-app/virtual-scroll/virtual-scroll-demo.scss
@@ -17,7 +17,7 @@
   .demo-item {
     -ms-writing-mode: tb-lr;
     -webkit-writing-mode: vertical-lr;
-    /* stylelint-disable-next-line material/no-prefixes */
+    // stylelint-disable-next-line material/no-prefixes
     writing-mode: vertical-lr;
   }
 }
diff --git a/src/material-experimental/mdc-button/fab.scss b/src/material-experimental/mdc-button/fab.scss
@@ -21,7 +21,7 @@
   // ```
   // However, Angular Material expects a `mat-icon` instead. The following
   // mixin will style the icons appropriately.
-  /* stylelint-disable-next-line selector-class-pattern */
+  // stylelint-disable-next-line selector-class-pattern
   .mat-icon, .material-icons {
     @include mdc-fab.icon_();
   }
@@ -30,7 +30,7 @@
 .mat-mdc-extended-fab {
   @include mdc-fab.extended_();
 
-  /* stylelint-disable-next-line selector-class-pattern */
+  // stylelint-disable-next-line selector-class-pattern
   .mat-icon, .material-icons {
     @include mdc-fab.extended-icon-padding(
       mdc-fab.$extended-icon-padding,
@@ -41,7 +41,7 @@
   // For Extended FAB with text label followed by icon.
   // We are checking for the a button class because white this is a FAB it
   // uses the same template as button.
-  /* stylelint-disable-next-line selector-class-pattern */
+  // stylelint-disable-next-line selector-class-pattern
   .mdc-button__label + .mat-icon, .mdc-button__label + .material-icons {
     @include mdc-fab.extended-icon-padding(
       mdc-fab.$extended-icon-padding,
@@ -50,4 +50,3 @@
     );
   }
 }
-
diff --git a/src/material-experimental/mdc-form-field/_mdc-text-field-structure-overrides.scss b/src/material-experimental/mdc-form-field/_mdc-text-field-structure-overrides.scss
@@ -68,7 +68,7 @@
   // placeholder if the form-field label is set to always float.
   // TODO(devversion): consider getting a mixin or variables for this (currently not available).
   // Stylelint no-prefixes rule disabled because MDC text-field only uses "::placeholder" too.
-  /* stylelint-disable-next-line material/no-prefixes */
+  // stylelint-disable-next-line material/no-prefixes
   .mat-mdc-form-field-label-always-float .mdc-text-field__input::placeholder {
     transition-delay: 40ms;
     transition-duration: 110ms;
diff --git a/src/material-experimental/mdc-helpers/_focus-indicators-theme.scss b/src/material-experimental/mdc-helpers/_focus-indicators-theme.scss
@@ -30,7 +30,7 @@
 ///   .demo-red-theme {
 ///     @include mat-mdc-strong-focus-indicators-theme(#f00);
 ///   }
-/* stylelint-disable-next-line material/theme-mixin-api */
+// stylelint-disable-next-line material/theme-mixin-api
 @mixin theme($theme-or-color) {
   @if meta.type-of($theme-or-color) != 'map' {
     @include _mat-mdc-strong-focus-indicators-border-color($theme-or-color);
diff --git a/src/material-experimental/mdc-helpers/_mdc-helpers.scss b/src/material-experimental/mdc-helpers/_mdc-helpers.scss
@@ -159,7 +159,7 @@ $mat-typography-2018-level-mappings: (
 
 // Configures MDC's global variables to reflect the given theme, applies the given styles,
 // then resets the global variables to prevent unintended side effects.
-/* stylelint-disable-next-line material/theme-mixin-api */
+// stylelint-disable-next-line material/theme-mixin-api
 @mixin mat-using-mdc-theme($config) {
   $primary: theming.get-color-from-palette(map.get($config, primary));
   $accent: theming.get-color-from-palette(map.get($config, accent));
@@ -250,7 +250,7 @@ $mat-typography-2018-level-mappings: (
 
 // Configures MDC's global variables to reflect the given typography config,
 // applies the given styles, then resets the global variables to prevent unintended side effects.
-/* stylelint-disable-next-line material/theme-mixin-api */
+// stylelint-disable-next-line material/theme-mixin-api
 @mixin mat-using-mdc-typography($config) {
   // Save the original values.
   $orig-mdc-typography-styles: mdc-typography.$styles;
diff --git a/src/material/autocomplete/autocomplete.scss b/src/material/autocomplete/autocomplete.scss
@@ -1,9 +1,7 @@
 @use '../core/style/menu-common';
 @use '../../cdk/a11y/a11y';
 
-/**
- * The max-height of the panel, currently matching mat-select value.
- */
+// The max-height of the panel, currently matching mat-select value.
 $panel-max-height: 256px !default;
 $panel-border-radius: 4px !default;
 
diff --git a/src/material/core/focus-indicators/_focus-indicators-theme.scss b/src/material/core/focus-indicators/_focus-indicators-theme.scss
@@ -23,7 +23,7 @@
 ///   .demo-red-theme {
 ///     @include mat-strong-focus-indicators-theme(#f00);
 ///   }
-/* stylelint-disable-next-line material/theme-mixin-api */
+// stylelint-disable-next-line material/theme-mixin-api
 @mixin theme($theme-or-color) {
   @if meta.type-of($theme-or-color) != 'map' {
     @include _mat-strong-focus-indicators-border-color($theme-or-color);
diff --git a/src/material/core/ripple/_ripple-theme.scss b/src/material/core/ripple/_ripple-theme.scss
@@ -2,7 +2,7 @@
 @use 'sass:meta';
 @use '../theming/theming';
 
-/* Colors for the ripple elements.*/
+// Colors for the ripple elements.
 @mixin color($config-or-theme) {
   $config: theming.get-color-config($config-or-theme);
   $foreground: map.get($config, foreground);
diff --git a/src/material/core/style/_menu-common.scss b/src/material/core/style/_menu-common.scss
@@ -3,7 +3,7 @@
 @use './layout-common';
 @use './vendor-prefixes';
 
-/** The mixins below are shared between mat-menu and mat-select */
+// The mixins below are shared between mat-menu and mat-select
 
 // menu width must be a multiple of 56px
 $overlay-min-width: 112px !default;   // 56 * 2
diff --git a/src/material/core/style/_vendor-prefixes.scss b/src/material/core/style/_vendor-prefixes.scss
@@ -1,4 +1,4 @@
-/* stylelint-disable material/no-prefixes */
+// stylelint-disable material/no-prefixes
 @mixin user-select($value) {
   -webkit-user-select: $value;
   -moz-user-select: $value;
@@ -43,4 +43,4 @@
   position: -webkit-sticky #{if($important, '!important', '')};
   position: sticky #{if($important, '!important', '')};
 }
-/* stylelint-enable */
+// stylelint-enable
diff --git a/src/material/core/typography/_typography.scss b/src/material/core/typography/_typography.scss
@@ -149,7 +149,7 @@
 }
 
 // Adds the base typography styles, based on a config.
-/* stylelint-disable-next-line material/theme-mixin-api */
+// stylelint-disable-next-line material/theme-mixin-api
 @mixin typography-hierarchy($config-or-theme, $selector: '.mat-typography') {
   $config: private-typography-to-2014-config(theming.get-typography-config($config-or-theme));
 
diff --git a/src/material/expansion/expansion-panel-header.scss b/src/material/expansion/expansion-panel-header.scss
@@ -66,10 +66,8 @@
   flex-grow: 2;
 }
 
-/**
- * Creates the expansion indicator arrow. Done using ::after rather than having
- * additional nodes in the template.
- */
+// Creates the expansion indicator arrow. Done using ::after
+// rather than havingadditional nodes in the template.
 .mat-expansion-indicator::after {
   border-style: solid;
   border-width: 0 2px 2px 0;
diff --git a/src/material/progress-spinner/progress-spinner.scss b/src/material/progress-spinner/progress-spinner.scss
@@ -87,36 +87,34 @@ $_mat-progress-spinner-default-circumference:
   $fallback-iterations: 4;
 
   @keyframes mat-progress-spinner-stroke-rotate-100 {
-    /*
-      stylelint-disable declaration-block-single-line-max-declarations,
-        declaration-block-semicolon-space-after
-    */
+    // stylelint-disable declaration-block-single-line-max-declarations
+    // stylelint-disable declaration-block-semicolon-space-after
 
     // .0001 percentage difference is necessary in order to avoid unwanted animation frames
     // for example because the animation duration is 4 seconds, .1% accounts to 4ms
     // which are enough to see the flicker described in
     // https://github.com/angular/components/issues/8984
     //
     // NOTE: this is replaced by js for any diameter other that 100
-    0%      { stroke-dashoffset: $start;  transform: rotate(0); }
-    12.5%   { stroke-dashoffset: $end;    transform: rotate(0); }
+    0%        { stroke-dashoffset: $start;  transform: rotate(0); }
+    12.5%     { stroke-dashoffset: $end;    transform: rotate(0); }
     12.5001%  { stroke-dashoffset: $end;    transform: rotateX(180deg) rotate(72.5deg); }
-    25%     { stroke-dashoffset: $start;  transform: rotateX(180deg) rotate(72.5deg); }
+    25%       { stroke-dashoffset: $start;  transform: rotateX(180deg) rotate(72.5deg); }
 
     25.0001%   { stroke-dashoffset: $start;  transform: rotate(270deg); }
-    37.5%   { stroke-dashoffset: $end;    transform: rotate(270deg); }
-    37.5001%  { stroke-dashoffset: $end;    transform: rotateX(180deg) rotate(161.5deg); }
-    50%     { stroke-dashoffset: $start;  transform: rotateX(180deg) rotate(161.5deg); }
+    37.5%      { stroke-dashoffset: $end;    transform: rotate(270deg); }
+    37.5001%   { stroke-dashoffset: $end;    transform: rotateX(180deg) rotate(161.5deg); }
+    50%        { stroke-dashoffset: $start;  transform: rotateX(180deg) rotate(161.5deg); }
 
     50.0001%  { stroke-dashoffset: $start;  transform: rotate(180deg); }
-    62.5%   { stroke-dashoffset: $end;    transform: rotate(180deg); }
+    62.5%     { stroke-dashoffset: $end;    transform: rotate(180deg); }
     62.5001%  { stroke-dashoffset: $end;    transform: rotateX(180deg) rotate(251.5deg); }
-    75%     { stroke-dashoffset: $start;  transform: rotateX(180deg) rotate(251.5deg); }
+    75%       { stroke-dashoffset: $start;  transform: rotateX(180deg) rotate(251.5deg); }
 
     75.0001%  { stroke-dashoffset: $start;  transform: rotate(90deg); }
-    87.5%   { stroke-dashoffset: $end;    transform: rotate(90deg); }
+    87.5%     { stroke-dashoffset: $end;    transform: rotate(90deg); }
     87.5001%  { stroke-dashoffset: $end;    transform: rotateX(180deg) rotate(341.5deg); }
-    100%    { stroke-dashoffset: $start;  transform: rotateX(180deg) rotate(341.5deg); }
+    100%      { stroke-dashoffset: $start;  transform: rotateX(180deg) rotate(341.5deg); }
     // stylelint-enable
   }
 
diff --git a/src/material/slide-toggle/slide-toggle.scss b/src/material/slide-toggle/slide-toggle.scss
@@ -59,7 +59,7 @@ $bar-track-width: $bar-width - $thumb-size;
   @include list-common.truncate-line();
 }
 
-/* If the label should be placed before the thumb then we just change the orders. */
+// If the label should be placed before the thumb then we just change the orders.
 .mat-slide-toggle-label-before {
   .mat-slide-toggle-label { order: 1; }
   .mat-slide-toggle-bar { order: 2; }
@@ -216,7 +216,7 @@ $bar-track-width: $bar-width - $thumb-size;
   }
 }
 
-/** Custom styling to make the slide-toggle usable in high contrast mode. */
+// Custom styling to make the slide-toggle usable in high contrast mode.
 @include a11y.high-contrast(active, off) {
   .mat-slide-toggle-thumb,
   .mat-slide-toggle-bar {
diff --git a/src/material/snack-bar/snack-bar-container.scss b/src/material/snack-bar/snack-bar-container.scss
@@ -24,10 +24,8 @@ $spacing-margin-handset: 8px !default;
   }
 }
 
-/**
- * The mat-snack-bar-handset class will be applied to the overlay
- * element causing styling to both it and the snack bar container.
- */
+// The mat-snack-bar-handset class will be applied to the overlay
+// element causing styling to both it and the snack bar container.
 .mat-snack-bar-handset {
   width: 100%;
 
diff --git a/src/material/table/_table-flex-styles.scss b/src/material/table/_table-flex-styles.scss
@@ -1,6 +1,4 @@
-/**
- * Flex-based table structure
- */
+// Flex-based table structure
 $header-row-height: 56px;
 $row-height: 48px;
 $row-horizontal-padding: 24px;
diff --git a/src/material/table/table.scss b/src/material/table/table.scss
@@ -3,9 +3,7 @@
 
 @include table-flex-styles.private-table-flex-styles();
 
-/**
- * Native HTML table structure
- */
+// Native HTML table structure
 table.mat-table {
   border-spacing: 0;
 }
diff --git a/src/material/toolbar/toolbar.scss b/src/material/toolbar/toolbar.scss
@@ -2,7 +2,7 @@
 
 $row-padding: 16px !default;
 
-/** @deprecated @breaking-change 8.0.0 */
+// @deprecated @breaking-change 8.0.0
 // TODO: Remove once internal g3 apps no longer depend on this variable. Tracked with: COMP-303.
 $height-mobile-portrait: 56px !default;
 
diff --git a/tools/stylelint/single-line-comment-only.ts b/tools/stylelint/single-line-comment-only.ts
@@ -0,0 +1,43 @@
+import {createPlugin, utils} from 'stylelint';
+import {basename} from 'path';
+
+const ruleName = 'material/single-line-comment-only';
+const messages = utils.ruleMessages(ruleName, {
+  expected: () => 'Multi-line comments are not allowed (e.g. /* */). ' +
+                  'Use single-line comments instead (//).',
+});
+
+/**
+ * Stylelint plugin that doesn't allow multi-line comments to
+ * be used, because they'll show up in the user's output.
+ */
+const plugin = createPlugin(ruleName, (isEnabled: boolean, options?: {filePattern?: string}) => {
+  return (root, result) => {
+    if (!isEnabled) {
+      return;
+    }
+
+    const filePattern = options?.filePattern ? new RegExp(options.filePattern) : null;
+
+    if (filePattern && !filePattern?.test(basename(root.source!.input.file!))) {
+      return;
+    }
+
+    root.walkComments(comment => {
+      // The `raws.inline` property isn't in the typing so we need to cast to any. Also allow
+      // comments starting with `!` since they're used to tell minifiers to preserve the comment.
+      if (!(comment.raws as any).inline && !comment.text.startsWith('!')) {
+        utils.report({
+          result,
+          ruleName,
+          message: messages.expected(),
+          node: comment
+        });
+      }
+    });
+  };
+});
+
+plugin.ruleName = ruleName;
+plugin.messages = messages;
+export default plugin;

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@`
`17`	`17`	`.demo-item {`
`18`	`18`	`-ms-writing-mode: tb-lr;`
`19`	`19`	`-webkit-writing-mode: vertical-lr;`
`20`		`- /* stylelint-disable-next-line material/no-prefixes */`
	`20`	`+ // stylelint-disable-next-line material/no-prefixes`
`21`	`21`	`writing-mode: vertical-lr;`
`22`	`22`	`}`
`23`	`23`	`}`