Upgrade: Reduce number of false-positive migrations of the important modifier (#14737)

The important candidate migration is one of the most broad we have since it matches for any utility that are prefixed with an exclamation mark.

When running the codemodes on our example projects, we noticed that this was instead creating false-positives with candidates used in code positions, e.g:

```ts
export default {
  shouldNotUse: !border.shouldUse,
}
```

To prevent false-positives, this PR adds a heuristics to detect wether or not a candidate is used in a non-code position. We do this by checking the character before and after the modifier and only allow quotes or spaces.

This can cause candidates to not migrate that are valid Tailwind CSS classes, e.g.:

```ts
let classNames = `!underline${isHovered ? ' font-bold' : ''}`
```

This, however, is not a big issue since v4 can parse the v3 important prefix too.

Author: philipp-spiess

CHANGELOG.md

@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - _Upgrade (experimental)_: Ensure legacy theme values ending in `1` (like `theme(spacing.1)`) are correctly migrated to custom properties ([#14724](https://github.com/tailwindlabs/tailwindcss/pull/14724))
 - _Upgrade (experimental)_: Migrate arbitrary values to bare values for the `from-*`, `via-*`, and `to-*` utilities ([#14725](https://github.com/tailwindlabs/tailwindcss/pull/14725))
 - _Upgrade (experimental)_: Ensure `layer(utilities)` is removed from `@import` to keep `@utility` top-level ([#14738](https://github.com/tailwindlabs/tailwindcss/pull/14738))
+- _Upgrade (experimental)_: Don't migrate important modifiers that are actually logical negations (e.g. `let foo = !border` to `let foo = border!`) ([#14737](https://github.com/tailwindlabs/tailwindcss/pull/14737))
 
 ### Changed
 

integrations/upgrade/js-config.test.ts

@@ -72,6 +72,11 @@ test(
         @tailwind components;
         @tailwind utilities;
       `,
+      'src/test.js': ts`
+        export default {
+          shouldNotUse: !border.shouldUse,
+        }
+      `,
       'node_modules/my-external-lib/src/template.html': html`
         <div class="text-red-500">
           Hello world!
@@ -82,7 +87,7 @@ test(
   async ({ exec, fs }) => {
     await exec('npx @tailwindcss/upgrade')
 
-    expect(await fs.dumpFiles('src/**/*.css')).toMatchInlineSnapshot(`
+    expect(await fs.dumpFiles('src/**/*.{css,js}')).toMatchInlineSnapshot(`
       "
       --- src/input.css ---
       @import 'tailwindcss';
@@ -134,6 +139,11 @@ test(
           }
         }
       }
+
+      --- src/test.js ---
+      export default {
+        shouldNotUse: !border.shouldUse,
+      }
       "
     `)
 

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

@@ -15,5 +15,39 @@ test.each([
     base: __dirname,
   })
 
-  expect(important(designSystem, {}, candidate)).toEqual(result)
+  expect(
+    important(designSystem, {}, candidate, {
+      contents: `"${candidate}"`,
+      start: 1,
+      end: candidate.length + 1,
+    }),
+  ).toEqual(result)
+})
+
+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')
+})
+
+test('does not match false positives with spaces at the end of the line', 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')
 })

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

@@ -19,9 +19,50 @@ export function important(
   designSystem: DesignSystem,
   _userConfig: Config,
   rawCandidate: string,
+  location?: {
+    contents: string
+    start: number
+    end: number
+  },
 ): string {
   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
+      // most programming languages. Since v4 is technically backward compatible
+      // with v3 in that it can read `!` in the front of the utility too, we err
+      // on the side of caution and only migrate candidates that we are certain
+      // are inside of a string.
+      if (location) {
+        let isQuoteBeforeCandidate = false
+        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
+          }
+        }
+
+        let isQuoteAfterCandidate = false
+        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
+          }
+        }
+
+        if (!isQuoteBeforeCandidate || !isQuoteAfterCandidate) {
+          continue
+        }
+      }
+
       // The printCandidate function will already put the exclamation mark in
       // the right place, so we just need to mark this candidate as requiring a
       // migration.
@@ -31,3 +72,14 @@ export function important(
 
   return rawCandidate
 }
+
+function isQuote(char: string) {
+  switch (char) {
+    case '"':
+    case "'":
+    case '`':
+      return true
+    default:
+      return false
+  }
+}

packages/@tailwindcss-upgrade/src/template/migrate.ts

@@ -17,6 +17,11 @@ export type Migration = (
   designSystem: DesignSystem,
   userConfig: Config,
   rawCandidate: string,
+  location?: {
+    contents: string
+    start: number
+    end: number
+  },
 ) => string
 
 export const DEFAULT_MIGRATIONS: Migration[] = [
@@ -34,9 +39,15 @@ export function migrateCandidate(
   designSystem: DesignSystem,
   userConfig: Config,
   rawCandidate: string,
+  // Location is only set when migrating a candidate from a source file
+  location?: {
+    contents: string
+    start: number
+    end: number
+  },
 ): string {
   for (let migration of DEFAULT_MIGRATIONS) {
-    rawCandidate = migration(designSystem, userConfig, rawCandidate)
+    rawCandidate = migration(designSystem, userConfig, rawCandidate, location)
   }
   return rawCandidate
 }
@@ -52,7 +63,11 @@ export default async function migrateContents(
   let changes: StringChange[] = []
 
   for (let { rawCandidate, start, end } of candidates) {
-    let migratedCandidate = migrateCandidate(designSystem, userConfig, rawCandidate)
+    let migratedCandidate = migrateCandidate(designSystem, userConfig, rawCandidate, {
+      contents,
+      start,
+      end,
+    })
 
     if (migratedCandidate === rawCandidate) {
       continue