Catch deprecated GHAE flags in feature-based versioning (github#31097)

Co-authored-by: Matt Pollard <mattpollard@users.noreply.github.com>

data/features/profile-name-enterprise-setting.yaml

@@ -1,4 +1,4 @@
 # Reference: Issue #6996 ability to show users' first/last name instead of username in issue/PR comment titles for public and internal repos
 versions:
   ghes: '>=3.6'
-  ghae: 'issue-6996'
+  ghae: '>=3.6'

lib/frontmatter.js

@@ -14,13 +14,6 @@ const layoutNames = [
   false,
 ]
 
-const semverValidRange = semver.validRange
-const semverRange = {
-  type: 'string',
-  conform: semverValidRange,
-  message: 'Must be a valid SemVer range',
-}
-
 const guideTypes = ['overview', 'quick_start', 'tutorial', 'how_to', 'reference']
 const featureVersions = fs
   .readdirSync(path.posix.join(process.cwd(), 'data/features'))
@@ -270,17 +263,29 @@ const featureVersionsProp = {
   },
 }
 
+const semverRange = {
+  type: 'string',
+  // TODO: The 'pattern' check below is a temporary check for presence of deprecated
+  // GHAE feature flags in FM. See details in docs-internal#29178.
+  // Due to a limitation of revalidator, this requires a temporary cleanup kludge
+  // in lib/temporary-ghae-deprecated-flag-error-cleanup.js.
+  // We can remove the 'pattern' check and that cleanup kludge after GHAE semantic versioning
+  // has been in place for a while, leaving only the 'conform' semver check.
+  pattern: /^(?!.*issue-\d+).*$/,
+  conform: semver.validRange,
+  messages: {
+    pattern: `Lightweight feature flags ('issue-NUMBER') are no longer supported in content. Use semantic versioning instead (ghae > 3.x or ghae: '> 3.x').`,
+    conform: 'Must be a valid SemVer range',
+  },
+}
+
 schema.properties.versions = {
   type: ['object', 'string'], // allow a '*' string to indicate all versions
   required: true,
-  // TODO - UNCOMMENT THE FOLLOWING LINE WHEN GHAE IS UPDATED WITH SEMVER VERSIONING
-  // additionalProperties: false, // don't allow any versions in FM that aren't defined in lib/all-versions
+  additionalProperties: false, // don't allow any versions in FM that aren't defined in lib/all-versions
   properties: Object.values(allVersions).reduce((acc, versionObj) => {
-    // TODO - REMOVE THIS IF STATEMENT WHEN GHAE IS UPDATED WITH SEMVER VERSIONING
-    if (!(versionObj.plan === 'github-ae' || versionObj.shortName === 'ghae')) {
-      acc[versionObj.plan] = semverRange
-      acc[versionObj.shortName] = semverRange
-    }
+    acc[versionObj.plan] = semverRange
+    acc[versionObj.shortName] = semverRange
     return acc
   }, featureVersionsProp),
 }

lib/read-frontmatter.js

@@ -1,6 +1,7 @@
 import matter from 'gray-matter'
 import revalidator from 'revalidator'
 import { difference, intersection } from 'lodash-es'
+import cleanUpDeprecatedGhaeFlagErrors from './temporary-ghae-deprecated-flag-error-cleanup.js'
 
 function readFrontmatter(markdown, opts = { validateKeyNames: false, validateKeyOrder: false }) {
   const schema = opts.schema || { properties: {} }
@@ -67,22 +68,9 @@ function readFrontmatter(markdown, opts = { validateKeyNames: false, validateKey
     errors.push(error)
   }
 
-  // TODO: Temporary check for presence of deprecated GHAE feature flags in FM.
-  // See details in docs-internal#29178.
-  // We can remove this after semantic versioning has been in place for a while.
-  // NOTE: This is here and not in the lib/frontmatter schema because the schema
-  // does a more general error check; this is an additional regex check for a
-  // specific error that contains a custom error message.
-  const ghaeVersion = data?.versions?.ghae || data?.versions?.['github-ae']
-  if (ghaeVersion && !filepath?.includes('/translations/')) {
-    if (/issue-\d+?/.test(ghaeVersion)) {
-      const error = {
-        property: 'versions',
-        message: `Lightweight feature flags ('${ghaeVersion}') are no longer supported in content. Use semantic versioning instead (ghae > 3.x or ghae: '> 3.x').`,
-        filepath,
-      }
-      errors.push(error)
-    }
+  // TODO temporary kludge! See notes in the module.
+  if (errors.length) {
+    errors = cleanUpDeprecatedGhaeFlagErrors(errors)
   }
 
   return { content, data, errors }

lib/temporary-ghae-deprecated-flag-error-cleanup.js

@@ -0,0 +1,27 @@
+// lib/frontmatter contains a temporary check for presence of deprecated GHAE
+// feature flags in FM. See details in docs-internal#29178.
+// TODO: Remove that check and this cleanup kludge after GHAE semantic versioning
+// has been in place for a while.
+//
+// We need this kludge because if lib/frontmatter finds an old flag using the
+// 'pattern' check, the semver 'conform' check will also fail. Showing both errors would
+// be confusing for contributors, so we want to only show the pattern failure because it has
+// a helpful customized message. Due to a limitation of revalidator, it's not possible
+// to prefer one error over the other programmatically. So this function deletes the
+// conform error if a pattern error is found.
+export default function cleanUpDeprecatedGhaeFlagErrors(errors) {
+  errors.forEach((error) => {
+    if (error.property === 'versions.ghae' && error.attribute === 'pattern') {
+      const currIndex = errors.indexOf(error)
+      const prevIndex = currIndex - 1 // Hack to get the conform error, which comes before this one.
+
+      // If this is a translated file, remove all errors on deprecated flags.
+      // If this is an English file, remove the conform error.
+      error.filepath?.includes('/translations/')
+        ? errors.splice(prevIndex, 2)
+        : errors.splice(prevIndex, 1)
+    }
+  })
+
+  return errors
+}

tests/linting/lint-versioning.js

@@ -10,6 +10,7 @@ import featureVersionsSchema from '../helpers/schemas/feature-versions-schema.js
 import walkFiles from '../../script/helpers/walk-files'
 import frontmatter from '../../lib/frontmatter.js'
 import loadSiteData from '../../lib/site-data.js'
+import cleanUpDeprecatedGhaeFlagErrors from '../../lib/temporary-ghae-deprecated-flag-error-cleanup.js'
 
 /*
   NOTE: This test suite does NOT validate the `versions` frontmatter in content files.
@@ -28,8 +29,13 @@ const allowedVersionNames = Object.keys(allVersionShortnames).concat(featureVers
 
 // Make sure data/features/*.yml contains valid versioning.
 describe('lint feature versions', () => {
-  test.each(featureVersions)('data/features/%s matches the schema', (_, featureVersion) => {
-    const { errors } = revalidator.validate(featureVersion, featureVersionsSchema)
+  test.each(featureVersions)('data/features/%s matches the schema', (name, featureVersion) => {
+    let { errors } = revalidator.validate(featureVersion, featureVersionsSchema)
+
+    // TODO temporary kludge! See notes in the module.
+    if (errors.length) {
+      errors = cleanUpDeprecatedGhaeFlagErrors(errors)
+    }
 
     const errorMessage = errors
       .map((error) => {