Skip to content

Commit 602070f

Browse files
tom-unfacebook-github-bot
tom-un
authored and
facebook-github-bot
committed
Add ES Lint rules for DynamicColorIOS()and ColorAndroid() (#28398)
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
1 parent 1281be6 commit 602070f

File tree

5 files changed

+195
-11
lines changed

5 files changed

+195
-11
lines changed

‎.eslintrc

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
rules: {
1414
'@react-native-community/no-haste-imports': 2,
1515
'@react-native-community/error-subclass-name': 2,
16+
'@react-native-community/platform-colors': 2,
1617
}
1718
},
1819
{

‎Libraries/StyleSheet/__tests__/normalizeColor-test.js

+12-11
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,6 @@
1313
const {OS} = require('../../Utilities/Platform');
1414
const normalizeColor = require('../normalizeColor');
1515

16-
const PlatformColorIOS = require('../PlatformColorValueTypes.ios')
17-
.PlatformColor;
18-
const DynamicColorIOS = require('../PlatformColorValueTypesIOS.ios')
19-
.DynamicColorIOS;
20-
const PlatformColorAndroid = require('../PlatformColorValueTypes.android')
21-
.PlatformColor;
22-
2316
describe('normalizeColor', function() {
2417
it('should accept only spec compliant colors', function() {
2518
expect(normalizeColor('#abc')).not.toBe(null);
@@ -139,8 +132,13 @@ describe('normalizeColor', function() {
139132

140133
describe('iOS', () => {
141134
if (OS === 'ios') {
135+
const PlatformColor = require('../PlatformColorValueTypes.ios')
136+
.PlatformColor;
137+
const DynamicColorIOS = require('../PlatformColorValueTypesIOS.ios')
138+
.DynamicColorIOS;
139+
142140
it('should normalize iOS PlatformColor colors', () => {
143-
const color = PlatformColorIOS('systemRedColor');
141+
const color = PlatformColor('systemRedColor');
144142
const normalizedColor = normalizeColor(color);
145143
const expectedColor = {semantic: ['systemRedColor']};
146144
expect(normalizedColor).toEqual(expectedColor);
@@ -155,8 +153,8 @@ describe('normalizeColor', function() {
155153

156154
it('should normalize iOS Dynamic colors with PlatformColor colors', () => {
157155
const color = DynamicColorIOS({
158-
light: PlatformColorIOS('systemBlackColor'),
159-
dark: PlatformColorIOS('systemWhiteColor'),
156+
light: PlatformColor('systemBlackColor'),
157+
dark: PlatformColor('systemWhiteColor'),
160158
});
161159
const normalizedColor = normalizeColor(color);
162160
const expectedColor = {
@@ -172,8 +170,11 @@ describe('normalizeColor', function() {
172170

173171
describe('Android', () => {
174172
if (OS === 'android') {
173+
const PlatformColor = require('../PlatformColorValueTypes.android')
174+
.PlatformColor;
175+
175176
it('should normalize Android PlatformColor colors', () => {
176-
const color = PlatformColorAndroid('?attr/colorPrimary');
177+
const color = PlatformColor('?attr/colorPrimary');
177178
const normalizedColor = normalizeColor(color);
178179
const expectedColor = {resource_paths: ['?attr/colorPrimary']};
179180
expect(normalizedColor).toEqual(expectedColor);

‎packages/eslint-plugin-react-native-community/__tests__/platform-colors-test.js

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @emails oncall+react_native
8+
* @format
9+
*/
10+
11+
'use strict';
12+
13+
const ESLintTester = require('./eslint-tester.js');
14+
15+
const rule = require('../platform-colors.js');
16+
17+
const eslintTester = new ESLintTester();
18+
19+
eslintTester.run('../platform-colors', rule, {
20+
valid: [
21+
"const color = PlatformColor('labelColor');",
22+
"const color = PlatformColor('controlAccentColor', 'controlColor');",
23+
"const color = DynamicColorIOS({light: 'black', dark: 'white'});",
24+
"const color = DynamicColorIOS({light: PlatformColor('black'), dark: PlatformColor('white')});",
25+
"const color = ColorAndroid('?attr/colorAccent')",
26+
],
27+
invalid: [
28+
{
29+
code: 'const color = PlatformColor();',
30+
errors: [{message: rule.meta.messages.platformColorArgsLength}],
31+
},
32+
{
33+
code:
34+
"const labelColor = 'labelColor'; const color = PlatformColor(labelColor);",
35+
errors: [{message: rule.meta.messages.platformColorArgTypes}],
36+
},
37+
{
38+
code:
39+
"const tuple = {light: 'black', dark: 'white'}; const color = DynamicColorIOS(tuple);",
40+
errors: [{message: rule.meta.messages.dynamicColorIOSArg}],
41+
},
42+
{
43+
code:
44+
"const black = 'black'; const color = DynamicColorIOS({light: black, dark: 'white'});",
45+
errors: [{message: rule.meta.messages.dynamicColorIOSLight}],
46+
},
47+
{
48+
code:
49+
"const white = 'white'; const color = DynamicColorIOS({light: 'black', dark: white});",
50+
errors: [{message: rule.meta.messages.dynamicColorIOSDark}],
51+
},
52+
{
53+
code: 'const color = ColorAndroid();',
54+
errors: [{message: rule.meta.messages.colorAndroidArg}],
55+
},
56+
{
57+
code:
58+
"const colorAccent = '?attr/colorAccent'; const color = ColorAndroid(colorAccent);",
59+
errors: [{message: rule.meta.messages.colorAndroidArg}],
60+
},
61+
],
62+
});

‎packages/eslint-plugin-react-native-community/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,5 @@
1010
exports.rules = {
1111
'error-subclass-name': require('./error-subclass-name'),
1212
'no-haste-imports': require('./no-haste-imports'),
13+
'platform-colors': require('./platform-colors'),
1314
};

‎packages/eslint-plugin-react-native-community/platform-colors.js

+119
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @format
8+
*/
9+
10+
module.exports = {
11+
meta: {
12+
type: 'problem',
13+
docs: {
14+
description:
15+
'Ensure that PlatformColor(), DynamicColorIOS(), and ColorAndroid() are passed literals of the expected shape.',
16+
},
17+
messages: {
18+
platformColorArgsLength:
19+
'PlatformColor() must have at least one argument that is a literal.',
20+
platformColorArgTypes:
21+
'PlatformColor() every argument must be a literal.',
22+
dynamicColorIOSArg:
23+
'DynamicColorIOS() must take a single argument of type Object containing two keys: light and dark.',
24+
dynamicColorIOSLight:
25+
'DynamicColorIOS() light value must be either a literal or a PlatformColor() call.',
26+
dynamicColorIOSDark:
27+
'DynamicColorIOS() dark value must be either a literal or a PlatformColor() call.',
28+
colorAndroidArg:
29+
'ColorAndroid() must take a single argument that is a literal.',
30+
},
31+
schema: [],
32+
},
33+
34+
create: function(context) {
35+
return {
36+
CallExpression: function(node) {
37+
if (node.callee.name === 'PlatformColor') {
38+
const args = node.arguments;
39+
if (args.length === 0) {
40+
context.report({
41+
node,
42+
messageId: 'platformColorArgsLength',
43+
});
44+
return;
45+
}
46+
if (!args.every(arg => arg.type === 'Literal')) {
47+
context.report({
48+
node,
49+
messageId: 'platformColorArgTypes',
50+
});
51+
return;
52+
}
53+
} else if (node.callee.name === 'DynamicColorIOS') {
54+
const args = node.arguments;
55+
if (!(args.length === 1 && args[0].type === 'ObjectExpression')) {
56+
context.report({
57+
node,
58+
messageId: 'dynamicColorIOSArg',
59+
});
60+
return;
61+
}
62+
const properties = args[0].properties;
63+
if (
64+
!(
65+
properties.length === 2 &&
66+
properties[0].type === 'Property' &&
67+
properties[0].key.name === 'light' &&
68+
properties[1].type === 'Property' &&
69+
properties[1].key.name === 'dark'
70+
)
71+
) {
72+
context.report({
73+
node,
74+
messageId: 'dynamicColorIOSArg',
75+
});
76+
return;
77+
}
78+
const light = properties[0];
79+
if (
80+
!(
81+
light.value.type === 'Literal' ||
82+
(light.value.type === 'CallExpression' &&
83+
light.value.callee.name === 'PlatformColor')
84+
)
85+
) {
86+
context.report({
87+
node,
88+
messageId: 'dynamicColorIOSLight',
89+
});
90+
return;
91+
}
92+
const dark = properties[1];
93+
if (
94+
!(
95+
dark.value.type === 'Literal' ||
96+
(dark.value.type === 'CallExpression' &&
97+
dark.value.callee.name === 'PlatformColor')
98+
)
99+
) {
100+
context.report({
101+
node,
102+
messageId: 'dynamicColorIOSDark',
103+
});
104+
return;
105+
}
106+
} else if (node.callee.name === 'ColorAndroid') {
107+
const args = node.arguments;
108+
if (!(args.length === 1 && args[0].type === 'Literal')) {
109+
context.report({
110+
node,
111+
messageId: 'colorAndroidArg',
112+
});
113+
return;
114+
}
115+
}
116+
},
117+
};
118+
},
119+
};

0 commit comments

Comments
 (0)