Upgrade: Redce number of false-positive migrations of the important modifier

Author: philipp-spiess

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - _Upgrade (experimental)_: Minify arbitrary values when printing candidates ([#14720](https://github.com/tailwindlabs/tailwindcss/pull/14720))
 - _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)_: Reduce the number of false-positive migration of the important modifier (e.g. `!border` to `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,25 @@ 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')
 })

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

@@ -19,9 +19,44 @@ 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
+      // to 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 charBefore = location.contents.at(location.start - 1)
+        let charAfter = location.contents.at(location.end)
+
+        if (
+          charBefore !== undefined &&
+          charBefore !== '"' &&
+          charBefore !== "'" &&
+          charBefore !== '`' &&
+          charBefore !== ' '
+        ) {
+          continue
+        }
+        if (
+          charAfter !== undefined &&
+          charAfter !== '"' &&
+          charAfter !== "'" &&
+          charAfter !== '`' &&
+          charAfter !== ' '
+        ) {
+          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.

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