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

Author: philipp-spiess

CHANGELOG.md

@@ -23,6 +23,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)) {
+  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
+      // 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 nextCandidate
+        }
+      }
+
       // 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