Skip to content

Commit

Permalink
Add ES Lint rules for DynamicColorIOS()and ColorAndroid() (#28398)
Browse files Browse the repository at this point in the history
Summary:
The [PlatformColor PR](#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: #28398

Test Plan: `yarn lint` passes.

Reviewed By: cpojer

Differential Revision: D20685383

Pulled By: TheSavior

fbshipit-source-id: 9bb37ccc059e74282b119577df0ced63cb9b1f53
  • Loading branch information
tom-un authored and facebook-github-bot committed Mar 28, 2020
1 parent 1281be6 commit 602070f
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 11 deletions.
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
},
{
Expand Down
23 changes: 12 additions & 11 deletions Libraries/StyleSheet/__tests__/normalizeColor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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 = {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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}],
},
],
});
1 change: 1 addition & 0 deletions packages/eslint-plugin-react-native-community/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
};
119 changes: 119 additions & 0 deletions packages/eslint-plugin-react-native-community/platform-colors.js
Original file line number Diff line number Diff line change
@@ -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;
}
}
},
};
},
};

0 comments on commit 602070f

Please sign in to comment.