Upgrade: Improve heuristics around important codemod (#14774)

This PR improves the heuristics around the important codemod (e.g.
`!border` => `border!`) as we noticed a few more cases where we the
current heuristics was not enough.

Specifically, we made it not migrate the candidate in the following
conditions:

- When there's an immediate property access: `{ "foo": !border.something
+ ""}`
- When it's used as condition in the template language: `<div
v-if="something && !border"></div>` or `<div x-if="!border"></div>`

## Test plan

I added test cases to the unit tests and updated the integration test to
contain a more sophisticated example.

---------

Co-authored-by: Adam Wathan <adam.wathan@gmail.com>

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Ensure individual logical property utilities are sorted later than left/right pair utilities ([#14777](https://github.com/tailwindlabs/tailwindcss/pull/14777))
+- Don't migrate important modifiers inside conditional statements in Vue and Alpine (e.g. `<div v-if="!border" />`) ([#14774](https://github.com/tailwindlabs/tailwindcss/pull/14774))
 - _Upgrade (experimental)_: Ensure `@import` statements for relative CSS files are actually migrated to use relative path syntax ([#14769](https://github.com/tailwindlabs/tailwindcss/pull/14769))
 
 ## [4.0.0-alpha.29] - 2024-10-23

integrations/upgrade/js-config.test.ts

@@ -123,9 +123,10 @@ test(
         @tailwind components;
         @tailwind utilities;
       `,
+      // prettier-ignore
       'src/test.js': ts`
         export default {
-          shouldNotUse: !border.shouldUse,
+          'shouldNotMigrate': !border.test + '',
         }
       `,
       'node_modules/my-external-lib/src/template.html': html`
@@ -275,7 +276,7 @@ test(
 
       --- src/test.js ---
       export default {
-        shouldNotUse: !border.shouldUse,
+        'shouldNotMigrate': !border.test + '',
       }
       "
     `)

packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts

@@ -38,16 +38,31 @@ test('does not match false positives', async () => {
   ).toEqual('!border')
 })
 
-test('does not match false positives with spaces at the end of the line', async () => {
+test('does not match false positives', async () => {
   let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', {
     base: __dirname,
   })
 
-  expect(
-    important(designSystem, {}, '!border', {
-      contents: `let notBorder = !border    \n`,
-      start: 16,
-      end: 16 + '!border'.length,
-    }),
-  ).toEqual('!border')
+  function shouldNotDetect(example: string, candidate = '!border') {
+    expect(
+      important(designSystem, {}, candidate, {
+        contents: example,
+        start: example.indexOf(candidate),
+        end: example.indexOf(candidate) + candidate.length,
+      }),
+    ).toEqual('!border')
+  }
+
+  shouldNotDetect(`let notBorder = !border    \n`)
+  shouldNotDetect(`{ "foo": !border.something + ""}\n`)
+  shouldNotDetect(`<div v-if="something && !border"></div>\n`)
+  shouldNotDetect(`<div v-else-if="something && !border"></div>\n`)
+  shouldNotDetect(`<div v-show="something && !border"></div>\n`)
+  shouldNotDetect(`<div v-if="!border || !border"></div>\n`)
+  shouldNotDetect(`<div v-else-if="!border || !border"></div>\n`)
+  shouldNotDetect(`<div v-show="!border || !border"></div>\n`)
+  shouldNotDetect(`<div v-if="!border"></div>\n`)
+  shouldNotDetect(`<div v-else-if="!border"></div>\n`)
+  shouldNotDetect(`<div v-show="!border"></div>\n`)
+  shouldNotDetect(`<div x-if="!border"></div>\n`)
 })

packages/@tailwindcss-upgrade/src/template/codemods/important.ts

@@ -3,6 +3,19 @@ import { parseCandidate } from '../../../../tailwindcss/src/candidate'
 import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
 import { printCandidate } from '../candidates'
 
+const QUOTES = ['"', "'", '`']
+const LOGICAL_OPERATORS = ['&&', '||', '===', '==', '!=', '!==', '>', '>=', '<', '<=']
+const CONDITIONAL_TEMPLATE_SYNTAX = [
+  // Vue
+  /v-else-if=['"]$/,
+  /v-if=['"]$/,
+  /v-show=['"]$/,
+
+  // Alpine
+  /x-if=['"]$/,
+  /x-show=['"]$/,
+]
+
 // In v3 the important modifier `!` sits in front of the utility itself, not
 // before any of the variants. In v4, we want it to be at the end of the utility
 // so that it's always in the same location regardless of whether you used
@@ -25,7 +38,7 @@ export function important(
     end: number
   },
 ): string {
-  for (let candidate of parseCandidate(rawCandidate, designSystem)) {
+  nextCandidate: for (let candidate of parseCandidate(rawCandidate, designSystem)) {
     if (candidate.important && candidate.raw[candidate.raw.length - 1] !== '!') {
       // The important migration is one of the most broad migrations with a high
       // potential of matching false positives since `!` is a valid character in
@@ -34,32 +47,54 @@ export function important(
       // on the side of caution and only migrate candidates that we are certain
       // are inside of a string.
       if (location) {
-        let isQuoteBeforeCandidate = false
+        let currentLineBeforeCandidate = ''
         for (let i = location.start - 1; i >= 0; i--) {
           let char = location.contents.at(i)!
           if (char === '\n') {
             break
           }
-          if (isQuote(char)) {
-            isQuoteBeforeCandidate = true
-            break
-          }
+          currentLineBeforeCandidate = char + currentLineBeforeCandidate
         }
-
-        let isQuoteAfterCandidate = false
+        let currentLineAfterCandidate = ''
         for (let i = location.end; i < location.contents.length; i++) {
           let char = location.contents.at(i)!
           if (char === '\n') {
             break
           }
-          if (isQuote(char)) {
-            isQuoteAfterCandidate = true
-            break
+          currentLineAfterCandidate += char
+        }
+
+        // Heuristic 1: Require the candidate to be inside quotes
+        let isQuoteBeforeCandidate = QUOTES.some((quote) =>
+          currentLineBeforeCandidate.includes(quote),
+        )
+        let isQuoteAfterCandidate = QUOTES.some((quote) =>
+          currentLineAfterCandidate.includes(quote),
+        )
+        if (!isQuoteBeforeCandidate || !isQuoteAfterCandidate) {
+          continue nextCandidate
+        }
+
+        // Heuristic 2: Disallow object access immediately following the candidate
+        if (currentLineAfterCandidate[0] === '.') {
+          continue nextCandidate
+        }
+
+        // Heuristic 3: Disallow logical operators preceding or following the candidate
+        for (let operator of LOGICAL_OPERATORS) {
+          if (
+            currentLineAfterCandidate.trim().startsWith(operator) ||
+            currentLineBeforeCandidate.trim().endsWith(operator)
+          ) {
+            continue nextCandidate
           }
         }
 
-        if (!isQuoteBeforeCandidate || !isQuoteAfterCandidate) {
-          continue
+        // Heuristic 4: Disallow conditional template syntax
+        for (let rule of CONDITIONAL_TEMPLATE_SYNTAX) {
+          if (rule.test(currentLineBeforeCandidate)) {
+            continue nextCandidate
+          }
         }
       }
 
@@ -72,14 +107,3 @@ export function important(
 
   return rawCandidate
 }
-
-function isQuote(char: string) {
-  switch (char) {
-    case '"':
-    case "'":
-    case '`':
-      return true
-    default:
-      return false
-  }
-}