From 602070f44b02220aeb036a7b3c26dad5c611b636 Mon Sep 17 00:00:00 2001 From: Tom Underhill Date: Fri, 27 Mar 2020 23:00:19 -0700 Subject: [PATCH] Add ES Lint rules for `DynamicColorIOS()`and `ColorAndroid()` (#28398) Summary: The [PlatformColor PR](https://github.com/facebook/react-native/pull/27908) added support for iOS and Android to express platform specific color values. The primary method for an app to specify such colors is via the `PlatformColor()` method that takes string arguments. The `PlatformColor` method returns an opaque Flow type enforcing that apps use the PlatformColor method instead of creating Objects from scratch -- doing so would make it harder to write static analysis tools around Color values in the future. But in addition to `PlatformColor()`, iOS has a `DynamicColorIOS()` method that takes an Object. The Flow type for this Object cannot be opaque, but we still want to enforce that app code doesn't pass variables instead of Object literals or that values in the Objects are variables. To ensure `DynamicColorIOS()` can be statically analyzed this change adds an ESLint rule to enforce that `DynamicColorIOS()` takes an Object literal of a specific shape. A `ColorAndroid()` was also introduced not for practical use but just to test having platform specific methods for more than one platform in the same app. A second ESLint rule is created for `ColorAndroid` as well. ## Changelog [General] [Changed] - Add ES Lint rules for `DynamicColorIOS()`and `ColorAndroid()` Pull Request resolved: https://github.com/facebook/react-native/pull/28398 Test Plan: `yarn lint` passes. Reviewed By: cpojer Differential Revision: D20685383 Pulled By: TheSavior fbshipit-source-id: 9bb37ccc059e74282b119577df0ced63cb9b1f53 --- .eslintrc | 1 + .../__tests__/normalizeColor-test.js | 23 ++-- .../__tests__/platform-colors-test.js | 62 +++++++++ .../index.js | 1 + .../platform-colors.js | 119 ++++++++++++++++++ 5 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 packages/eslint-plugin-react-native-community/__tests__/platform-colors-test.js create mode 100644 packages/eslint-plugin-react-native-community/platform-colors.js diff --git a/.eslintrc b/.eslintrc index 9385a7481c3ce6..7795e95b40cd3c 100644 --- a/.eslintrc +++ b/.eslintrc @@ -13,6 +13,7 @@ rules: { '@react-native-community/no-haste-imports': 2, '@react-native-community/error-subclass-name': 2, + '@react-native-community/platform-colors': 2, } }, { diff --git a/Libraries/StyleSheet/__tests__/normalizeColor-test.js b/Libraries/StyleSheet/__tests__/normalizeColor-test.js index 2a127626b692a3..12eac4cfbf8d6d 100644 --- a/Libraries/StyleSheet/__tests__/normalizeColor-test.js +++ b/Libraries/StyleSheet/__tests__/normalizeColor-test.js @@ -13,13 +13,6 @@ const {OS} = require('../../Utilities/Platform'); const normalizeColor = require('../normalizeColor'); -const PlatformColorIOS = require('../PlatformColorValueTypes.ios') - .PlatformColor; -const DynamicColorIOS = require('../PlatformColorValueTypesIOS.ios') - .DynamicColorIOS; -const PlatformColorAndroid = require('../PlatformColorValueTypes.android') - .PlatformColor; - describe('normalizeColor', function() { it('should accept only spec compliant colors', function() { expect(normalizeColor('#abc')).not.toBe(null); @@ -139,8 +132,13 @@ describe('normalizeColor', function() { describe('iOS', () => { if (OS === 'ios') { + const PlatformColor = require('../PlatformColorValueTypes.ios') + .PlatformColor; + const DynamicColorIOS = require('../PlatformColorValueTypesIOS.ios') + .DynamicColorIOS; + it('should normalize iOS PlatformColor colors', () => { - const color = PlatformColorIOS('systemRedColor'); + const color = PlatformColor('systemRedColor'); const normalizedColor = normalizeColor(color); const expectedColor = {semantic: ['systemRedColor']}; expect(normalizedColor).toEqual(expectedColor); @@ -155,8 +153,8 @@ describe('normalizeColor', function() { it('should normalize iOS Dynamic colors with PlatformColor colors', () => { const color = DynamicColorIOS({ - light: PlatformColorIOS('systemBlackColor'), - dark: PlatformColorIOS('systemWhiteColor'), + light: PlatformColor('systemBlackColor'), + dark: PlatformColor('systemWhiteColor'), }); const normalizedColor = normalizeColor(color); const expectedColor = { @@ -172,8 +170,11 @@ describe('normalizeColor', function() { describe('Android', () => { if (OS === 'android') { + const PlatformColor = require('../PlatformColorValueTypes.android') + .PlatformColor; + it('should normalize Android PlatformColor colors', () => { - const color = PlatformColorAndroid('?attr/colorPrimary'); + const color = PlatformColor('?attr/colorPrimary'); const normalizedColor = normalizeColor(color); const expectedColor = {resource_paths: ['?attr/colorPrimary']}; expect(normalizedColor).toEqual(expectedColor); diff --git a/packages/eslint-plugin-react-native-community/__tests__/platform-colors-test.js b/packages/eslint-plugin-react-native-community/__tests__/platform-colors-test.js new file mode 100644 index 00000000000000..9d1168272ecdc6 --- /dev/null +++ b/packages/eslint-plugin-react-native-community/__tests__/platform-colors-test.js @@ -0,0 +1,62 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails oncall+react_native + * @format + */ + +'use strict'; + +const ESLintTester = require('./eslint-tester.js'); + +const rule = require('../platform-colors.js'); + +const eslintTester = new ESLintTester(); + +eslintTester.run('../platform-colors', rule, { + valid: [ + "const color = PlatformColor('labelColor');", + "const color = PlatformColor('controlAccentColor', 'controlColor');", + "const color = DynamicColorIOS({light: 'black', dark: 'white'});", + "const color = DynamicColorIOS({light: PlatformColor('black'), dark: PlatformColor('white')});", + "const color = ColorAndroid('?attr/colorAccent')", + ], + invalid: [ + { + code: 'const color = PlatformColor();', + errors: [{message: rule.meta.messages.platformColorArgsLength}], + }, + { + code: + "const labelColor = 'labelColor'; const color = PlatformColor(labelColor);", + errors: [{message: rule.meta.messages.platformColorArgTypes}], + }, + { + code: + "const tuple = {light: 'black', dark: 'white'}; const color = DynamicColorIOS(tuple);", + errors: [{message: rule.meta.messages.dynamicColorIOSArg}], + }, + { + code: + "const black = 'black'; const color = DynamicColorIOS({light: black, dark: 'white'});", + errors: [{message: rule.meta.messages.dynamicColorIOSLight}], + }, + { + code: + "const white = 'white'; const color = DynamicColorIOS({light: 'black', dark: white});", + errors: [{message: rule.meta.messages.dynamicColorIOSDark}], + }, + { + code: 'const color = ColorAndroid();', + errors: [{message: rule.meta.messages.colorAndroidArg}], + }, + { + code: + "const colorAccent = '?attr/colorAccent'; const color = ColorAndroid(colorAccent);", + errors: [{message: rule.meta.messages.colorAndroidArg}], + }, + ], +}); diff --git a/packages/eslint-plugin-react-native-community/index.js b/packages/eslint-plugin-react-native-community/index.js index aac041d58e1879..22d01cce278dc2 100644 --- a/packages/eslint-plugin-react-native-community/index.js +++ b/packages/eslint-plugin-react-native-community/index.js @@ -10,4 +10,5 @@ exports.rules = { 'error-subclass-name': require('./error-subclass-name'), 'no-haste-imports': require('./no-haste-imports'), + 'platform-colors': require('./platform-colors'), }; diff --git a/packages/eslint-plugin-react-native-community/platform-colors.js b/packages/eslint-plugin-react-native-community/platform-colors.js new file mode 100644 index 00000000000000..4d20496f8772d8 --- /dev/null +++ b/packages/eslint-plugin-react-native-community/platform-colors.js @@ -0,0 +1,119 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +module.exports = { + meta: { + type: 'problem', + docs: { + description: + 'Ensure that PlatformColor(), DynamicColorIOS(), and ColorAndroid() are passed literals of the expected shape.', + }, + messages: { + platformColorArgsLength: + 'PlatformColor() must have at least one argument that is a literal.', + platformColorArgTypes: + 'PlatformColor() every argument must be a literal.', + dynamicColorIOSArg: + 'DynamicColorIOS() must take a single argument of type Object containing two keys: light and dark.', + dynamicColorIOSLight: + 'DynamicColorIOS() light value must be either a literal or a PlatformColor() call.', + dynamicColorIOSDark: + 'DynamicColorIOS() dark value must be either a literal or a PlatformColor() call.', + colorAndroidArg: + 'ColorAndroid() must take a single argument that is a literal.', + }, + schema: [], + }, + + create: function(context) { + return { + CallExpression: function(node) { + if (node.callee.name === 'PlatformColor') { + const args = node.arguments; + if (args.length === 0) { + context.report({ + node, + messageId: 'platformColorArgsLength', + }); + return; + } + if (!args.every(arg => arg.type === 'Literal')) { + context.report({ + node, + messageId: 'platformColorArgTypes', + }); + return; + } + } else if (node.callee.name === 'DynamicColorIOS') { + const args = node.arguments; + if (!(args.length === 1 && args[0].type === 'ObjectExpression')) { + context.report({ + node, + messageId: 'dynamicColorIOSArg', + }); + return; + } + const properties = args[0].properties; + if ( + !( + properties.length === 2 && + properties[0].type === 'Property' && + properties[0].key.name === 'light' && + properties[1].type === 'Property' && + properties[1].key.name === 'dark' + ) + ) { + context.report({ + node, + messageId: 'dynamicColorIOSArg', + }); + return; + } + const light = properties[0]; + if ( + !( + light.value.type === 'Literal' || + (light.value.type === 'CallExpression' && + light.value.callee.name === 'PlatformColor') + ) + ) { + context.report({ + node, + messageId: 'dynamicColorIOSLight', + }); + return; + } + const dark = properties[1]; + if ( + !( + dark.value.type === 'Literal' || + (dark.value.type === 'CallExpression' && + dark.value.callee.name === 'PlatformColor') + ) + ) { + context.report({ + node, + messageId: 'dynamicColorIOSDark', + }); + return; + } + } else if (node.callee.name === 'ColorAndroid') { + const args = node.arguments; + if (!(args.length === 1 && args[0].type === 'Literal')) { + context.report({ + node, + messageId: 'colorAndroidArg', + }); + return; + } + } + }, + }; + }, +};