Repository: postcss/autoprefixer (js)

Issue:
[css-grid] Report a warning if duplicate area names are detected
Take this css:

````css
.grid-alpha {
  grid-template-areas: "delta  echo";
}

.grid-beta {
  grid-template-areas: "echo  delta";
}

.grid-cell {
  -ms-grid-column: ???; /* what column does .grid-cell go in? */
  arid-area: echo;
}
````

Autoprefixer has no access to the DOM so it doesn't really know what area `.grid-cell` belongs to.

Currently Autoprefixer silently just goes with whatever the last `grid-template-areas` definition was in the style-sheet (in this case `"echo  delta"`). If a user places `.grid-cell` in `.grid-alpha` then it will look great in modern browsers but be placed in the wrong column in IE.

Since it is silent, a user could end up building their whole site blissfully unaware of this issue, using duplicate area names everywhere, then finally open IE to see their whole layout go to shit.

Autoprefixer should detect if there are any area name conflicts and if so, warn users to use unique area names at all times.

Pull request:
Show warning if grid-area has a non-unique identifier (fixes #1038)
This is my first PR which adds warning when grid-area has a non-unique identifier (conflict of names). #1038

Not sure if code is clean enough, but I had a lot of fun working on this issue. Thanks! :)

Some questions:
* In my code I refer to area names in grid-area as "identifiers" ([see this](https://developer.mozilla.org/en-US/docs/Web/CSS/custom-ident)). Is it alright?
* How should we handle grid-area inside media? I've omitted checking warnings inside media for now

Patch:
diff --git a/lib/hacks/grid-template-areas.js b/lib/hacks/grid-template-areas.js
index 6506182..42aab18 100644
--- a/lib/hacks/grid-template-areas.js
+++ b/lib/hacks/grid-template-areas.js
@@ -5,7 +5,8 @@ const {
   prefixTrackProp,
   prefixTrackValue,
   getGridGap,
-  warnGridGap
+  warnGridGap,
+  warnDuplicateNames
 } = require('./grid-utils')
 
 function getGridRows (tpl) {
@@ -70,6 +71,9 @@ class GridTemplateAreas extends Declaration {
         result
       })
 
+      // warn if grid-template-areas has a duplicate area name
+      warnDuplicateNames({ decl, result })
+
       const areas = parseGridAreas({
         rows: gridRows,
         gap
diff --git a/lib/hacks/grid-template.js b/lib/hacks/grid-template.js
index 74c2823..c56c2e8 100644
--- a/lib/hacks/grid-template.js
+++ b/lib/hacks/grid-template.js
@@ -3,7 +3,8 @@ const {
   parseTemplate,
   insertAreas,
   getGridGap,
-  warnGridGap
+  warnGridGap,
+  warnDuplicateNames
 } = require('./grid-utils')
 
 class GridTemplate extends Declaration {
@@ -42,6 +43,9 @@ class GridTemplate extends Declaration {
         result
       })
 
+      // warn if grid-template has a duplicate area name
+      warnDuplicateNames({ decl, result })
+
       if ((hasRows && hasColumns) || hasAreas) {
         decl.cloneBefore({
           prop: '-ms-grid-rows',
diff --git a/lib/hacks/grid-utils.js b/lib/hacks/grid-utils.js
index ce38e52..ce55a5f 100644
--- a/lib/hacks/grid-utils.js
+++ b/lib/hacks/grid-utils.js
@@ -1,4 +1,5 @@
 const parser = require('postcss-value-parser')
+const list = require('postcss').list
 
 function convert (value) {
   if (value &&
@@ -139,6 +140,96 @@ function prefixTrackValue ({ value, gap }) {
   return parser.stringify(result)
 }
 
+/**
+ * Parse grid areas from declaration
+ * @param Declaration [decl]
+ * @return Array<String>
+ */
+function parseGridTemplateStrings (decl) {
+  const template = parseTemplate({ decl, gap: getGridGap(decl) })
+  return Object.keys(template.areas)
+}
+
+/**
+ * Walk through every grid-template(-areas)
+ * declaration and try to find non-unique area names (duplicates)
+ * @param  {Declaration} decl
+ * @param  {Result} result
+ * @return {void}
+ */
+function warnDuplicateNames ({ decl, result }) {
+  const rule = decl.parent
+  const inMediaRule = !!getParentMedia(rule)
+  const areas = parseGridTemplateStrings(decl)
+  const root = decl.root()
+
+  // stop if no areas found
+  if (areas.length === 0) {
+    return false
+  }
+
+  root.walkDecls(/grid-template(-areas)?$/g, d => {
+    const nodeRule = d.parent
+    const nodeRuleMedia = getParentMedia(nodeRule)
+    const isEqual = rule.toString() === nodeRule.toString()
+    const foundAreas = parseGridTemplateStrings(d)
+    const maxIndex = Math.max(root.index(nodeRule), root.index(nodeRuleMedia))
+    const isLookingDown = root.index(rule) < maxIndex
+
+    // skip node if no grid areas is found
+    if (foundAreas.length === 0) {
+      return true
+    }
+
+    // abort if we outside media rule and walking below our rule
+    // (we need to check ONLY the rules that are above)
+    if (!inMediaRule && isLookingDown) {
+      return false
+    }
+
+    // abort if we're in the same rule
+    if (isEqual) {
+      return false
+    }
+
+    // if we're inside media rule, we need to compare selectors
+    if (inMediaRule) {
+      const selectors = list.comma(nodeRule.selector)
+      const selectorIsFound = selectors.some(sel => {
+        if (sel === rule.selector) {
+          // compare selectors as a comma list
+          return true
+        } else if (nodeRule.selector === rule.selector) {
+          // compare selectors as a whole (e.g. ".i, .j" === ".i, .j")
+          return true
+        }
+        return false
+      })
+
+      // stop walking if we found the selector
+      if (selectorIsFound) {
+        return false
+      }
+    }
+
+    const duplicates = areas.filter(area => foundAreas.includes(area))
+    if (duplicates.length > 0) {
+      const word = duplicates.length > 1 ? 'names' : 'name'
+      decl.warn(
+        result,
+        ['',
+          `  duplicate area ${ word } detected in rule: ${ rule.selector }`,
+          `  duplicate area ${ word }: ${ duplicates.join(', ') }`,
+          `  duplicate area names cause unexpected behavior in IE`
+        ].join('\n')
+      )
+      return false
+    }
+    return undefined
+  })
+  return undefined
+}
+
 // Parse grid-template-areas
 
 const DOTS = /^\.+$/
@@ -407,5 +498,6 @@ module.exports = {
   prefixTrackProp,
   prefixTrackValue,
   getGridGap,
-  warnGridGap
+  warnGridGap,
+  warnDuplicateNames
 }
diff --git a/package.json b/package.json
index c0b94d4..a1f073d 100644
--- a/package.json
+++ b/package.json
@@ -12,6 +12,7 @@
   "author": "Andrey Sitnik <andrey@sitnik.ru>",
   "license": "MIT",
   "repository": "postcss/autoprefixer",
+  "homepage": "https://github.com/postcss/autoprefixer#readme",
   "engines": {
     "node": ">=6.0.0"
   },


Test patch:
diff --git a/test/autoprefixer.test.js b/test/autoprefixer.test.js
index 7f23042..e525197 100644
--- a/test/autoprefixer.test.js
+++ b/test/autoprefixer.test.js
@@ -553,6 +553,47 @@ describe('hacks', () => {
     expect(result.warnings()).toEqual([])
   })
 
+  it('shows warning if grid-template has a duplicate area name', () => {
+    const input = read('grid-template')
+    const output = read('grid-template.out')
+    const instance = prefixer('grid-area')
+    const result = postcss([instance]).process(input)
+
+    expect(result.css).toEqual(output)
+    expect(result.warnings().map(i => i.toString())).toEqual([
+      [
+        `autoprefixer: <css input>:41:5: `,
+        `  duplicate area names detected in rule: .f`,
+        `  duplicate area names: head, nav, main, foot`,
+        `  duplicate area names cause unexpected behavior in IE`
+      ].join('\n'),
+      [
+        `autoprefixer: <css input>:133:5: `,
+        `  duplicate area names detected in rule: .g-conflict`,
+        `  duplicate area names: g, h`,
+        `  duplicate area names cause unexpected behavior in IE`
+      ].join('\n'),
+      [
+        `autoprefixer: <css input>:154:5: `,
+        `  duplicate area names detected in rule: .g-conflict-2`,
+        `  duplicate area names: g, h`,
+        `  duplicate area names cause unexpected behavior in IE`
+      ].join('\n'),
+      [
+        `autoprefixer: <css input>:186:5: `,
+        `  duplicate area names detected in rule: .k`,
+        `  duplicate area names: i, j`,
+        `  duplicate area names cause unexpected behavior in IE`
+      ].join('\n'),
+      [
+        `autoprefixer: <css input>:228:5: `,
+        `  duplicate area name detected in rule: .z`,
+        `  duplicate area name: m`,
+        `  duplicate area names cause unexpected behavior in IE`
+      ].join('\n')
+    ])
+  })
+
   it('ignores values for CSS3PIE props', () => {
     const css = read('pie')
     expect(postcss([compiler]).process(css).css).toEqual(css)
diff --git a/test/cases/grid-template.css b/test/cases/grid-template.css
index 5d37a93..74059d1 100644
--- a/test/cases/grid-template.css
+++ b/test/cases/grid-template.css
@@ -115,3 +115,146 @@
 .cell-F {
     grid-area: f;
 }
+
+.g {
+    display: grid;
+    grid-gap: 10px;
+    grid-template:
+        "g   g" 100px
+        "g   g" 100px
+        "h   h" 100px /
+        1fr  1fr;
+}
+
+/* Should trigger a warning */
+.g-conflict {
+    display: grid;
+    grid-gap: 10px;
+    grid-template:
+        "g   g" 100px
+        "g   g" 100px
+        "h   h" 100px /
+        1fr  1fr;
+}
+
+@media (min-width: 600px) {
+  /* This should *not* trigger a warning */
+  .g {
+    display: grid;
+    grid-gap: 10px;
+    grid-template:
+        "g   h" 100px /
+        1fr  1fr;
+  }
+
+  /* This *should* trigger a warning */
+  .g-conflict-2 {
+    display: grid;
+    grid-gap: 10px;
+    grid-template:
+        "g   h" 100px /
+        1fr  1fr;
+  }
+}
+
+/* comma list tests */
+/* None of these should throw any warnings (unless specified) */
+.i, .j {
+  display: grid;
+  grid-gap: 10px;
+  grid-template:
+      "i    j" 100px /
+      1fr  1fr;
+}
+
+@media (max-width: 600px) {
+  .i {
+    grid-template:
+      "i" 100px
+      "j" 100px /
+      1fr;
+  }
+  .j {
+    grid-template:
+      "i" 100px
+      "j" 100px /
+      1fr;
+  }
+
+  /* This one should throw a warning */
+  .k {
+    grid-template:
+      "i" 100px
+      "j" 100px /
+      1fr;
+  }
+}
+
+@media (min-width: 900px) {
+  .i, .j {
+    display: grid;
+    grid-gap: 10px;
+    grid-template:
+      "i" 100px
+      "j" 100px /
+      1fr;
+  }
+}
+
+/* media query test */
+@media (min-width: 601px) {
+  .l {
+    display: grid;
+    grid-gap: 10px;
+    grid-template:
+        "l   m" 100px /
+        1fr  1fr;
+  }
+}
+
+@media (max-width: 600px) {
+  .l {
+    display: grid;
+    grid-gap: 10px;
+    grid-template:
+        "l   m" 100px /
+        1fr  1fr;
+  }
+
+  /* this should display a warning */
+  .z {
+    display: grid;
+    grid-gap: 10px;
+    grid-template:
+        "m   z" 100px /
+        1fr  1fr;
+  }
+}
+
+.k-1 {
+    grid-area: g;
+}
+
+.k-2 {
+    grid-area: h;
+}
+
+.k-3 {
+    grid-area: i;
+}
+
+.k-4 {
+    grid-area: j;
+}
+
+.k-6 {
+    grid-area: l;
+}
+
+.k-6 {
+    grid-area: m;
+}
+
+.k-7 {
+    grid-area: z;
+}
diff --git a/test/cases/grid-template.out.css b/test/cases/grid-template.out.css
index 3ea243c..3260401 100644
--- a/test/cases/grid-template.out.css
+++ b/test/cases/grid-template.out.css
@@ -211,3 +211,267 @@
         -ms-grid-column: 5;
     }
 }
+
+.g {
+    display: -ms-grid;
+    display: grid;
+    grid-gap: 10px;
+    -ms-grid-rows: 100px 10px 100px 10px 100px;
+    -ms-grid-columns: 1fr 10px 1fr;
+    grid-template:
+        "g   g" 100px
+        "g   g" 100px
+        "h   h" 100px /
+        1fr  1fr;
+}
+
+/* Should trigger a warning */
+.g-conflict {
+    display: -ms-grid;
+    display: grid;
+    grid-gap: 10px;
+    -ms-grid-rows: 100px 10px 100px 10px 100px;
+    -ms-grid-columns: 1fr 10px 1fr;
+    grid-template:
+        "g   g" 100px
+        "g   g" 100px
+        "h   h" 100px /
+        1fr  1fr;
+}
+
+@media (min-width: 600px) {
+  /* This should *not* trigger a warning */
+  .g {
+    display: -ms-grid;
+    display: grid;
+    grid-gap: 10px;
+    -ms-grid-rows: 100px;
+    -ms-grid-columns: 1fr 10px 1fr;
+    grid-template:
+        "g   h" 100px /
+        1fr  1fr;
+  }
+
+  /* This *should* trigger a warning */
+  .g-conflict-2 {
+    display: -ms-grid;
+    display: grid;
+    grid-gap: 10px;
+    -ms-grid-rows: 100px;
+    -ms-grid-columns: 1fr 10px 1fr;
+    grid-template:
+        "g   h" 100px /
+        1fr  1fr;
+  }
+}
+
+/* comma list tests */
+/* None of these should throw any warnings (unless specified) */
+.i, .j {
+  display: -ms-grid;
+  display: grid;
+  grid-gap: 10px;
+  -ms-grid-rows: 100px;
+  -ms-grid-columns: 1fr 10px 1fr;
+  grid-template:
+      "i    j" 100px /
+      1fr  1fr;
+}
+
+@media (max-width: 600px) {
+  .i {
+    -ms-grid-rows: 100px 100px;
+    -ms-grid-columns: 1fr;
+    grid-template:
+      "i" 100px
+      "j" 100px /
+      1fr;
+  }
+  .j {
+    -ms-grid-rows: 100px 100px;
+    -ms-grid-columns: 1fr;
+    grid-template:
+      "i" 100px
+      "j" 100px /
+      1fr;
+  }
+
+  /* This one should throw a warning */
+  .k {
+    -ms-grid-rows: 100px 100px;
+    -ms-grid-columns: 1fr;
+    grid-template:
+      "i" 100px
+      "j" 100px /
+      1fr;
+  }
+}
+
+@media (min-width: 900px) {
+  .i, .j {
+    display: -ms-grid;
+    display: grid;
+    grid-gap: 10px;
+    -ms-grid-rows: 100px 10px 100px;
+    -ms-grid-columns: 1fr;
+    grid-template:
+      "i" 100px
+      "j" 100px /
+      1fr;
+  }
+}
+
+/* media query test */
+@media (min-width: 601px) {
+  .l {
+    display: -ms-grid;
+    display: grid;
+    grid-gap: 10px;
+    -ms-grid-rows: 100px;
+    -ms-grid-columns: 1fr 10px 1fr;
+    grid-template:
+        "l   m" 100px /
+        1fr  1fr;
+  }
+}
+
+@media (max-width: 600px) {
+  .l {
+    display: -ms-grid;
+    display: grid;
+    grid-gap: 10px;
+    -ms-grid-rows: 100px;
+    -ms-grid-columns: 1fr 10px 1fr;
+    grid-template:
+        "l   m" 100px /
+        1fr  1fr;
+  }
+
+  /* this should display a warning */
+  .z {
+    display: -ms-grid;
+    display: grid;
+    grid-gap: 10px;
+    -ms-grid-rows: 100px;
+    -ms-grid-columns: 1fr 10px 1fr;
+    grid-template:
+        "m   z" 100px /
+        1fr  1fr;
+  }
+}
+
+.k-1 {
+    -ms-grid-row: 1;
+    -ms-grid-row-span: 3;
+    -ms-grid-column: 1;
+    -ms-grid-column-span: 3;
+    grid-area: g;
+}
+
+.k-2 {
+    -ms-grid-row: 5;
+    -ms-grid-column: 1;
+    -ms-grid-column-span: 3;
+    grid-area: h;
+}
+
+@media (min-width: 600px) {
+  .k-1 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 1;
+  }
+  .k-2 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 3;
+  }
+}
+
+@media (min-width: 600px) {
+  .k-1 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 1;
+  }
+  .k-2 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 3;
+  }
+}
+
+.k-3 {
+    -ms-grid-row: 1;
+    -ms-grid-column: 1;
+    grid-area: i;
+}
+
+.k-4 {
+    -ms-grid-row: 1;
+    -ms-grid-column: 3;
+    grid-area: j;
+}
+
+@media (min-width: 900px) {
+  .k-3 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 1;
+  }
+  .k-4 {
+        -ms-grid-row: 3;
+        -ms-grid-column: 1;
+  }
+}
+
+@media (max-width: 600px) {
+  .k-3 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 1;
+  }
+  .k-4 {
+        -ms-grid-row: 2;
+        -ms-grid-column: 1;
+  }
+}
+
+.k-6 {
+    grid-area: l;
+}
+
+.k-6 {
+    grid-area: m;
+}
+
+@media (max-width: 600px) {
+  .k-6 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 1;
+  }
+  .k-6 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 3;
+  }
+}
+
+@media (min-width: 601px) {
+  .k-6 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 1;
+  }
+  .k-6 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 3;
+  }
+}
+
+.k-7 {
+    grid-area: z;
+}
+
+@media (max-width: 600px) {
+  .k-6 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 1;
+  }
+  .k-7 {
+        -ms-grid-row: 1;
+        -ms-grid-column: 3;
+  }
+}