[WIP] SolidJS Components return once

crates/biome_diagnostics_categories/src/categories.rs

@@ -107,19 +107,20 @@ define_categories! {
     "lint/correctness/useValidForDirection": "https://biomejs.dev/linter/rules/use-valid-for-direction",
     "lint/correctness/useYield": "https://biomejs.dev/linter/rules/use-yield",
     "lint/nursery/colorNoInvalidHex": "https://biomejs.dev/linter/rules/color-no-invalid-hex",
+    "lint/nursery/componentsReturnOnce": "https://biomejs.dev/linter/rules/components-return-once",
     "lint/nursery/noApproximativeNumericConstant": "https://biomejs.dev/linter/rules/no-approximative-numeric-constant",
     "lint/nursery/noBarrelFile": "https://biomejs.dev/linter/rules/no-barrel-file",
     "lint/nursery/noColorInvalidHex": "https://biomejs.dev/linter/rules/no-color-invalid-hex",
     "lint/nursery/noConsole": "https://biomejs.dev/linter/rules/no-console",
     "lint/nursery/noDoneCallback": "https://biomejs.dev/linter/rules/no-done-callback",
     "lint/nursery/noDuplicateElseIf": "https://biomejs.dev/linter/rules/no-duplicate-else-if",
+    "lint/nursery/noDuplicateFontNames": "https://biomejs.dev/linter/rules/no-font-family-duplicate-names",
     "lint/nursery/noDuplicateJsonKeys": "https://biomejs.dev/linter/rules/no-duplicate-json-keys",
     "lint/nursery/noDuplicateTestHooks": "https://biomejs.dev/linter/rules/no-duplicate-test-hooks",
     "lint/nursery/noEvolvingAny": "https://biomejs.dev/linter/rules/no-evolving-any",
     "lint/nursery/noExcessiveNestedTestSuites": "https://biomejs.dev/linter/rules/no-excessive-nested-test-suites",
     "lint/nursery/noExportsInTest": "https://biomejs.dev/linter/rules/no-exports-in-test",
     "lint/nursery/noFocusedTests": "https://biomejs.dev/linter/rules/no-focused-tests",
-    "lint/nursery/noDuplicateFontNames": "https://biomejs.dev/linter/rules/no-font-family-duplicate-names",
     "lint/nursery/noMisplacedAssertion": "https://biomejs.dev/linter/rules/no-misplaced-assertion",
     "lint/nursery/noNamespaceImport": "https://biomejs.dev/linter/rules/no-namespace-import",
     "lint/nursery/noNodejsModules": "https://biomejs.dev/linter/rules/no-nodejs-modules",

crates/biome_js_analyze/src/lint/nursery.rs

@@ -2,6 +2,7 @@
 
 use biome_analyze::declare_group;
 
+pub mod components_return_once;
 pub mod no_barrel_file;
 pub mod no_console;
 pub mod no_done_callback;
@@ -29,6 +30,7 @@ declare_group! {
     pub Nursery {
         name : "nursery" ,
         rules : [
+            self :: components_return_once :: ComponentsReturnOnce ,
             self :: no_barrel_file :: NoBarrelFile ,
             self :: no_console :: NoConsole ,
             self :: no_done_callback :: NoDoneCallback ,

crates/biome_js_analyze/src/lint/nursery/components_return_once.rs

@@ -0,0 +1,119 @@
+use biome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
+use biome_js_syntax::{AnyJsFunction, JsReturnStatement};
+
+declare_rule! {
+    /// Disallow early returns in components.
+    ///
+    /// Solid components only run once, and so conditionals should be inside JSX.
+    ///
+    /// https://github.com/solidjs-community/eslint-plugin-solid/blob/main/docs/components-return-once.md
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// function Component() {
+    ///   if (condition) {
+    ///     return <div />;
+    ///   }
+    ///   return <span />;
+    /// }
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```js
+    /// function Component() {
+    ///   return <div />;
+    /// }
+    /// ```
+    ///
+    pub ComponentsReturnOnce {
+        version: "next",
+        name: "componentsReturnOnce",
+        // TODO: eslint plugin solid source
+        recommended: false,
+    }
+}
+
+/// Returns `true` if the function is (probably) a component (because it is PascalCase)
+fn is_component_name(name: &str) -> bool {
+    name.chars()
+        .next()
+        .map(|x| x.is_uppercase())
+        .unwrap_or_default()
+}
+
+pub enum ReturnType {
+    Conditional,
+    EarlyReturn,
+}
+
+impl ReturnType {
+    pub fn get_message(&self) -> &str {
+        match self {
+            Self::Conditional => "Solid components run once, so a conditional return breaks reactivity. Move the condition inside a JSX element, such as a fragment or <Show />.",
+            Self::EarlyReturn =>  "Solid components run once, so an early return breaks reactivity. Move the condition inside a JSX element, such as a fragment or <Show />."
+        }
+    }
+}
+
+impl Rule for ComponentsReturnOnce {
+    type Query = Ast<AnyJsFunction>;
+    type State = Vec<(ReturnType, JsReturnStatement)>;
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let func = ctx.query();
+
+        if let Some(binding) = func.binding() {
+            if let Some(ident) = binding.as_js_identifier_binding() {
+                if let Ok(name) = ident.name_token() {
+                    if !is_component_name(name.text()) {
+                        // NOTE: Only consider components, not normal functions
+                        return None;
+                    }
+                }
+            }
+        }
+
+        let body = func.body().ok()?;
+        let body = body.as_js_function_body()?;
+
+        let mut returns = vec![];
+
+        for statement in body.statements() {
+            if let Some(ret) = statement.as_js_return_statement() {
+                if let Some(arg) = ret.argument() {
+                    if arg.as_js_conditional_expression().is_some() {
+                        returns.push((ReturnType::Conditional, ret.to_owned()));
+                    }
+                }
+            }
+        }
+
+        Some(returns)
+    }
+
+    fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
+        let mut issues = state.iter();
+
+        if let Some((return_type, statement)) = issues.next() {
+            let span = statement.return_token().ok()?.text_range();
+
+            let mut diagnostic =
+                RuleDiagnostic::new(rule_category!(), span, return_type.get_message());
+
+            for (return_type, statement) in issues {
+                let span = statement.return_token().unwrap().text_range();
+                diagnostic = diagnostic.detail(span, return_type.get_message());
+            }
+
+            Some(diagnostic)
+        } else {
+            None
+        }
+    }
+}

crates/biome_js_analyze/src/options.rs

@@ -2,6 +2,8 @@
 
 use crate::lint;
 
+pub type ComponentsReturnOnce =
+    <lint::nursery::components_return_once::ComponentsReturnOnce as biome_analyze::Rule>::Options;
 pub type NoAccessKey = <lint::a11y::no_access_key::NoAccessKey as biome_analyze::Rule>::Options;
 pub type NoAccumulatingSpread = < lint :: performance :: no_accumulating_spread :: NoAccumulatingSpread as biome_analyze :: Rule > :: Options ;
 pub type NoApproximativeNumericConstant = < lint :: suspicious :: no_approximative_numeric_constant :: NoApproximativeNumericConstant as biome_analyze :: Rule > :: Options ;

crates/biome_js_analyze/tests/specs/nursery/componentsReturnOnce/invalid.js

@@ -0,0 +1,54 @@
+function Component() {
+  if (condition) {
+    return <div />;
+  }
+  return <span />;
+};
+
+const Component = () => {
+  if (condition) {
+    return <div />;
+  }
+  return <span />;
+};
+
+function Component() {
+  return Math.random() > 0.5 ? <div>Big!</div> : <div>Small!</div>;
+};
+
+function Component() {
+  return Math.random() > 0.5 ? <div>Big!</div> : "Small!";
+};
+
+function Component() {
+  return Math.random() > 0.5 ? (
+    <div>Big! No, really big!</div>
+  ) : (
+    <div>Small!</div>
+  );
+};
+
+function Component(props) {
+  return props.cond1 ? (
+    <div>Condition 1</div>
+  ) : Boolean(props.cond2) ? (
+    <div>Not condition 1, but condition 2</div>
+  ) : (
+    <div>Neither condition 1 or 2</div>
+  );
+};
+
+function Component(props) {
+  return !!props.cond && <div>Conditional</div>;
+};
+
+function Component(props) {
+  return props.primary || <div>{props.secondaryText}</div>;
+};
+
+HOC(() => {
+  if (condition) {
+    return <div />;
+  }
+  return <div />;
+});

crates/biome_js_analyze/tests/specs/nursery/componentsReturnOnce/valid.js

@@ -0,0 +1,52 @@
+function Component() {
+  return <div />;
+};
+
+function someFunc() {
+  if (condition) {
+    return 5;
+  }
+  return 10;
+}
+
+function notAComponent() {
+  if (condition) {
+    return <div />;
+  }
+  return <div />;
+}
+
+callback(() => {
+  if (condition) {
+    return <div />;
+  }
+  return <div />;
+});
+
+function Component() {
+  const renderContent = () => {
+    if (false) return <></>;
+    return <></>;
+  };
+  return <>{renderContent()}</>;
+}
+
+function Component() {
+  function renderContent() {
+    if (false) return <></>;
+    return <></>;
+  }
+  return <>{renderContent()}</>;
+}
+
+function Component() {
+  const renderContent = () => {
+    const renderContentInner = () => {
+      // ifs in render functions are fine no matter what nesting level this is
+      if (false) return;
+      return <></>;
+    };
+    return <>{renderContentInner()}</>;
+  };
+  return <></>;
+}

crates/biome_js_analyze/tests/specs/nursery/componentsReturnOnce/valid.js.snap

@@ -0,0 +1,60 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: valid.js
+---
+# Input
+```jsx
+function Component() {
+  return <div />;
+};
+
+function someFunc() {
+  if (condition) {
+    return 5;
+  }
+  return 10;
+}
+
+function notAComponent() {
+  if (condition) {
+    return <div />;
+  }
+  return <div />;
+}
+
+callback(() => {
+  if (condition) {
+    return <div />;
+  }
+  return <div />;
+});
+
+function Component() {
+  const renderContent = () => {
+    if (false) return <></>;
+    return <></>;
+  };
+  return <>{renderContent()}</>;
+}
+
+function Component() {
+  function renderContent() {
+    if (false) return <></>;
+    return <></>;
+  }
+  return <>{renderContent()}</>;
+}
+
+function Component() {
+  const renderContent = () => {
+    const renderContentInner = () => {
+      // ifs in render functions are fine no matter what nesting level this is
+      if (false) return;
+      return <></>;
+    };
+    return <>{renderContentInner()}</>;
+  };
+  return <></>;
+}
+
+```