rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2760083701
##########
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:
This is a direct TypeScript migration of the existing JavaScript code. The
suggestion is valid but out of scope for this PR - the ESLint rule's behavior
was intentionally preserved as-is. A follow-up PR could enhance the rule's
coverage.
##########
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:
This is a direct TypeScript migration of the existing JavaScript code. The
suggestion is valid but out of scope for this PR - the ESLint rule's behavior
was intentionally preserved as-is. A follow-up PR could enhance the rule's
coverage.
##########
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:
This is a direct TypeScript migration of the existing JavaScript code. The
suggestion is valid but out of scope for this PR - the ESLint rule's behavior
was intentionally preserved as-is. A follow-up PR could enhance the rule's
coverage.
--
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]
