rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2758094935
##########
superset-frontend/eslint-rules/eslint-plugin-i18n-strings/index.ts:
##########
@@ -22,21 +22,26 @@
* @author Apache
*/
+import type { Rule } from 'eslint';
+import type { Node } from 'estree';
+
//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
-/** @type {import('eslint').Rule.RuleModule} */
-module.exports = {
+const plugin: { rules: Record<string, Rule.RuleModule> } = {
rules: {
'no-template-vars': {
- create(context) {
- function handler(node) {
- if (node.arguments.length) {
- const firstArgs = node.arguments[0];
+ create(context: Rule.RuleContext): Rule.RuleListener {
+ function handler(node: Node): void {
+ const callNode = node as Node & {
+ arguments: Array<Node & { expressions?: Node[] }>;
+ };
+ if (callNode.arguments.length) {
+ const firstArgs = callNode.arguments[0];
if (
Review Comment:
Fixed in 79e7d816cc. The rule now iterates over all arguments instead of
just the first, using a for loop with early break once a violation is found.
##########
superset-frontend/eslint-rules/eslint-plugin-i18n-strings/index.ts:
##########
@@ -76,29 +84,39 @@ module.exports = {
// Check for Button components
if (parent.type === 'JSXExpressionContainer') {
- const jsx = parent.parent;
+ const jsx = (parent as Node & { parent?: Node }).parent as
+ | (Node & {
+ openingElement?: { name: { name: string } };
+ })
+ | undefined;
if (jsx?.type === 'JSXElement') {
- const elementName = jsx.openingElement.name.name;
+ const elementName = jsx.openingElement?.name.name;
return elementName === 'Button';
}
}
return false;
}
- function handler(node) {
- if (node.arguments.length) {
- const firstArg = node.arguments[0];
+ function handler(node: Node): void {
+ const callNode = node as Node & {
+ arguments: Array<Node & { value?: string }>;
+ };
+ if (callNode.arguments.length) {
+ const firstArg = callNode.arguments[0];
if (
firstArg.type === 'Literal' &&
typeof firstArg.value === 'string'
) {
Review Comment:
Fixed in 79e7d816cc. The sentence-case-buttons rule now also iterates over
all arguments to catch Title Case in any position.
##########
superset-frontend/eslint-rules/eslint-plugin-theme-colors/index.ts:
##########
@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @fileoverview Rule to warn about literal colors
+ * @author Apache
+ */
+
+import type { Rule } from 'eslint';
+import type { Node, SourceLocation } from 'estree';
+
+const COLOR_KEYWORDS: string[] = require('./colors');
+
+function hasHexColor(quasi: string): boolean {
+ const regex = /#([a-f0-9]{3}|[a-f0-9]{4}(?:[a-f0-9]{2}){0,2})\b/gi;
+ return !!quasi.match(regex);
+}
+
+function hasRgbColor(quasi: string): boolean {
+ const regex = /rgba?\((\d+),\s*(\d+),\s*(\d+)(?:,\s*(\d+(?:\.\d+)?))?\)/i;
+ return !!quasi.match(regex);
+}
+
+function hasLiteralColor(quasi: string, strict: boolean = false): boolean {
+ // matches literal colors at the start or end of a CSS prop
+ return COLOR_KEYWORDS.some((color: string) => {
+ const regexColon = new RegExp(`: ${color}`);
+ const regexSemicolon = new RegExp(` ${color};`);
+ return (
+ !!quasi.match(regexColon) ||
+ !!quasi.match(regexSemicolon) ||
+ (strict && quasi === color)
+ );
+ });
+}
+
+const WARNING_MESSAGE: string =
+ 'Theme color variables are preferred over rgb(a)/hex/literal colors';
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+interface TemplateElementNode extends Node {
+ value?: { raw: string };
+ parent?: Node & {
+ parent?: Node & { loc?: SourceLocation | null };
+ };
+}
+
+interface LiteralNode extends Node {
+ value?: string;
+ parent?: Node;
+}
+
+const plugin: { rules: Record<string, Rule.RuleModule> } = {
+ rules: {
+ 'no-literal-colors': {
+ create(context: Rule.RuleContext): Rule.RuleListener {
+ const warned: string[] = [];
+ return {
+ TemplateElement(node: Node): void {
+ const templateNode = node as TemplateElementNode;
+ const rawValue = templateNode?.value?.raw;
+ const isChildParentTagged =
+ templateNode?.parent?.parent?.type ===
'TaggedTemplateExpression';
+ const isChildParentArrow =
+ templateNode?.parent?.parent?.type === 'ArrowFunctionExpression';
+ const isParentTemplateLiteral =
+ templateNode?.parent?.type === 'TemplateLiteral';
+ const loc = templateNode?.parent?.parent?.loc;
+ const locId = loc && JSON.stringify(loc);
+ const hasWarned = locId ? warned.includes(locId) : false;
+ if (
+ !hasWarned &&
+ (isChildParentTagged ||
+ (isChildParentArrow && isParentTemplateLiteral)) &&
+ rawValue &&
+ (hasLiteralColor(rawValue) ||
+ hasHexColor(rawValue) ||
+ hasRgbColor(rawValue))
+ ) {
+ context.report(node, loc as SourceLocation, WARNING_MESSAGE);
+ if (locId) {
+ warned.push(locId);
+ }
+ }
+ },
+ Literal(node: Node): void {
+ const literalNode = node as LiteralNode;
+ const value = literalNode?.value;
+ const isParentProperty = literalNode?.parent?.type === 'Property';
+ const locId = JSON.stringify(node.loc);
+ const hasWarned = warned.includes(locId);
+
+ if (
Review Comment:
Fixed in 79e7d816cc. Added an early return when `typeof value \!== 'string'`
to skip non-string literals (numbers, booleans, null, RegExp).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
