Add hasFeature function and disallow features.has (#4506)

The problem with `features.has` is that it takes
a string isntead of a `GPUFeatureName`. To prevent
typos when checking for features, add a lint warning
for `features.has(feature)` and suggest using
`hasFeature(features, feature)`. `hasFeature`
takes a `GPUFeatureName` and so will warn if the
string pass is not a valid feature.

Author: greggman

.eslintrc.json

@@ -58,6 +58,11 @@
             "message": "Use requestDeviceTracked() instead of requestDevice().",
             // We don't seem to need direct calls to requestDevice() at all so we can just disallow all of them.
             "selector": "CallExpression > MemberExpression > Identifier[name=\"requestDevice\"]"
+          },
+          {
+            "message": "Use hasFeature() instead of features.has().",
+            // features.has takes any string. We want only valid feature names.
+            "selector": "CallExpression[callee.property.name='has'][callee.object.name='features'],CallExpression[callee.property.name='has'][callee.object.property.name='features']"
           }
         ],
 

src/common/framework/test_config.ts

@@ -1,4 +1,4 @@
-import { assert } from '../util/util.js';
+import { assert, hasFeature } from '../util/util.js';
 
 export type TestConfig = {
   /**
@@ -90,7 +90,7 @@ export const globalTestConfig: TestConfig = {
 // is trying to test that compatibility devices have the correct validation.
 export function isCompatibilityDevice(device: GPUDevice) {
   if (globalTestConfig.compatibility) {
-    assert(!device.features.has('core-features-and-limits'));
+    assert(!hasFeature(device.features, 'core-features-and-limits'));
   }
   return globalTestConfig.compatibility;
 }

src/common/util/navigator_gpu.ts

@@ -2,7 +2,7 @@
 import { TestCaseRecorder } from '../framework/fixture.js';
 import { globalTestConfig } from '../framework/test_config.js';
 
-import { ErrorWithExtra, assert, objectEquals } from './util.js';
+import { ErrorWithExtra, assert, hasFeature, objectEquals } from './util.js';
 
 /**
  * Finds and returns the `navigator.gpu` object (or equivalent, for non-browser implementations).
@@ -164,7 +164,9 @@ export function getGPU(recorder: TestCaseRecorder | null): GPU {
           Object.defineProperty(adapter, 'features', {
             enumerable: false,
             value: new Set(
-              adapter.features.has('core-features-and-limits') ? ['core-features-and-limits'] : []
+              hasFeature(adapter.features, 'core-features-and-limits')
+                ? ['core-features-and-limits']
+                : []
             ),
           });
         }
@@ -177,6 +179,7 @@ export function getGPU(recorder: TestCaseRecorder | null): GPU {
         for (const [feature] of desc.requiredFeatures) {
           // Note: This adapter has had its features property over-ridden and will only return
           // have nothing or 'core-features-and-limits'.
+          // eslint-disable-next-line no-restricted-syntax
           if (!adapter.features.has(feature)) {
             throw new TypeError(`requested feature ${feature} does not exist on adapter`);
           }

src/common/util/util.ts

@@ -496,3 +496,12 @@ export function filterUniqueValueTestVariants(valueTestVariants: ValueTestVarian
 export function makeValueTestVariant(base: number, variant: ValueTestVariant) {
   return base * variant.mult + variant.add;
 }
+
+/**
+ * Use instead of features.has because feature's has takes any string
+ * and we want to prevent typos.
+ */
+export function hasFeature(features: GPUSupportedFeatures, feature: GPUFeatureName) {
+  // eslint-disable-next-line no-restricted-syntax
+  return features.has(feature);
+}

src/resources/cache/hashes.json

@@ -6,7 +6,7 @@ import { Fixture } from '../../../../common/framework/fixture.js';
 import { makeTestGroup } from '../../../../common/framework/test_group.js';
 import { keysOf } from '../../../../common/util/data_tables.js';
 import { getGPU } from '../../../../common/util/navigator_gpu.js';
-import { assert, objectEquals } from '../../../../common/util/util.js';
+import { assert, hasFeature, objectEquals } from '../../../../common/util/util.js';
 import { isPowerOfTwo } from '../../../util/math.js';
 
 export const g = makeTestGroup(Fixture);
@@ -158,7 +158,7 @@ If they exist, they must both exist and be powers of two, and
     // Once 'subgroups' lands, the properties should be defined with default values 4 and 128
     // when adapter does not support the feature.
     // https://github.com/gpuweb/gpuweb/pull/4963
-    if (adapter.features.has('subgroups')) {
+    if (hasFeature(adapter.features, 'subgroups')) {
       t.expect(
         subgroupMinSize !== undefined,
         'GPUAdapterInfo.subgroupMinSize must exist when subgroups supported'

src/webgpu/api/operation/adapter/info.spec.ts

@@ -8,7 +8,7 @@ potentially limited native resources.
 import { Fixture } from '../../../../common/framework/fixture.js';
 import { makeTestGroup } from '../../../../common/framework/test_group.js';
 import { getGPU } from '../../../../common/util/navigator_gpu.js';
-import { assert, assertReject, typedEntries } from '../../../../common/util/util.js';
+import { assert, assertReject, hasFeature, typedEntries } from '../../../../common/util/util.js';
 import {
   getDefaultLimitsForCTS,
   kFeatureNames,
@@ -44,7 +44,7 @@ g.test('default')
 
     if (device.features.size === 1) {
       t.expect(
-        device.features.has('core-features-and-limits'),
+        hasFeature(device.features, 'core-features-and-limits'),
         'Default device should not have any features other than "core-features-and-limits"'
       );
     } else {
@@ -204,9 +204,9 @@ g.test('features,known')
     assert(adapter !== null);
 
     const promise = t.requestDeviceTracked(adapter, { requiredFeatures: [feature] });
-    if (adapter.features.has(feature)) {
+    if (hasFeature(adapter.features, feature)) {
       const device = await promise;
-      t.expect(device.features.has(feature), 'Device should include the required feature');
+      t.expect(hasFeature(device.features, feature), 'Device should include the required feature');
     } else {
       t.shouldReject('TypeError', promise);
     }
@@ -496,12 +496,12 @@ g.test('always_returns_device')
       const device = await t.requestDeviceTracked(adapter);
       assert(device instanceof GPUDevice, 'requestDevice must return a device or throw');
 
-      if (featureLevel === 'core' && adapter.features.has('core-features-and-limits')) {
+      if (featureLevel === 'core' && hasFeature(adapter.features, 'core-features-and-limits')) {
         // Check if the device supports core, when featureLevel is core and adapter supports core.
         // This check is to make sure something lower-level is not forcing compatibility mode.
 
         t.expect(
-          device.features.has('core-features-and-limits'),
+          hasFeature(device.features, 'core-features-and-limits'),
           'must not get a Compatibility adapter if not requested'
         );
       }

src/webgpu/api/operation/adapter/requestDevice.spec.ts

@@ -103,6 +103,7 @@ changes to WebGPU do not break sites requesting everything.
 
     // Test that all the adapter features are on the device.
     for (const feature of t.adapter.features) {
+      // eslint-disable-next-line no-restricted-syntax
       t.expect(t.device.features.has(feature), `device has feature: ${feature}`);
     }
   });