Skip to content

Commit

Permalink
fix(lint/useExportType): preserve leading comments (#2688)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos authored May 3, 2024
1 parent 7656d1b commit ac22955
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 14 deletions.
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,22 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

Contributed by @Conaclos

- [useExportType](https://biomejs.dev/linter/rules/use-export-type/) no longer removes leading comments ([#2685](https://github.com/biomejs/biome/issues/2685)).

Previously, `useExportType` removed leading comments when it factorized the `type` qualifier.
It now provides a code fix that preserves the leading comments:

```diff
- export {
+ export type {
/**leading comment*/
- type T
+ T
}
```

Contributed by @Conaclos

- Fix typo by renaming `useConsistentBuiltinInstatiation` to `useConsistentBuiltinInstantiation`
Contributed by @minht11

Expand Down
30 changes: 16 additions & 14 deletions crates/biome_js_analyze/src/lint/style/use_export_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use biome_diagnostics::Applicability;
use biome_js_factory::make;
use biome_js_syntax::{AnyJsExportNamedSpecifier, JsExportNamedClause, JsFileSource, T};
use biome_rowan::{
trim_leading_trivia_pieces, AstNode, AstSeparatedList, BatchMutationExt, TriviaPieceKind,
chain_trivia_pieces, trim_leading_trivia_pieces, AstNode, AstSeparatedList, BatchMutationExt,
TriviaPieceKind,
};

declare_rule! {
Expand Down Expand Up @@ -106,11 +107,11 @@ impl Rule for UseExportType {
}
}
if exports_only_types {
Some(ExportTypeFix::GroupTypeExports)
Some(ExportTypeFix::UseExportType)
} else if specifiers_requiring_type_marker.is_empty() {
None
} else {
Some(ExportTypeFix::AddInlineType(
Some(ExportTypeFix::AddInlineTypeQualifiers(
specifiers_requiring_type_marker,
))
}
Expand All @@ -119,13 +120,13 @@ impl Rule for UseExportType {
fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let range = ctx.query().range();
let (diagnostic_range, diagnostic_message) = match state {
ExportTypeFix::GroupTypeExports => (
ExportTypeFix::UseExportType => (
range,
markup! {
"All exports are only types and should thus use "<Emphasis>"export type"</Emphasis>"."
},
),
ExportTypeFix::AddInlineType(specifiers) => {
ExportTypeFix::AddInlineTypeQualifiers(specifiers) => {
let range = specifiers
.iter()
.map(|x| x.range())
Expand All @@ -152,17 +153,18 @@ impl Rule for UseExportType {
let export_named_clause = ctx.query();
let mut mutation = ctx.root().begin();
let diagnostic = match state {
ExportTypeFix::GroupTypeExports => {
ExportTypeFix::UseExportType => {
let specifier_list = export_named_clause.specifiers();
let mut new_specifiers = Vec::new();
for specifier in specifier_list.iter().filter_map(|x| x.ok()) {
if let Some(type_token) = specifier.type_token() {
let mut new_specifier = specifier.with_type_token(None);
if type_token.has_trailing_comments() {
new_specifier = new_specifier.prepend_trivia_pieces(
let new_specifier = specifier
.with_type_token(None)
.trim_leading_trivia()?
.prepend_trivia_pieces(chain_trivia_pieces(
type_token.leading_trivia().pieces(),
trim_leading_trivia_pieces(type_token.trailing_trivia().pieces()),
)?;
}
))?;
new_specifiers.push(new_specifier);
} else {
new_specifiers.push(specifier)
Expand Down Expand Up @@ -193,7 +195,7 @@ impl Rule for UseExportType {
mutation,
}
}
ExportTypeFix::AddInlineType(specifiers) => {
ExportTypeFix::AddInlineTypeQualifiers(specifiers) => {
for specifier in specifiers {
mutation.replace_node(
specifier.clone(),
Expand Down Expand Up @@ -227,6 +229,6 @@ pub enum ExportTypeFix {
/**
* Group inline type exports such as `export { type A, type B }` into `export type { A, B }`.
*/
GroupTypeExports,
AddInlineType(Vec<AnyJsExportNamedSpecifier>),
UseExportType,
AddInlineTypeQualifiers(Vec<AnyJsExportNamedSpecifier>),
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,11 @@ export { Interface, TypeAlias, Enum, func as f, Class };
export /*0*/ { /*1*/ type /*2*/ func /*3*/, /*4*/ type Class as C /*5*/ } /*6*/;

export {}

import { type T7, type T8 } from "./mod.ts";
export {
/*1*/
type T7,
/*2*/
type T8,
};
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ export /*0*/ { /*1*/ type /*2*/ func /*3*/, /*4*/ type Class as C /*5*/ } /*6*/;

export {}

import { type T7, type T8 } from "./mod.ts";
export {
/*1*/
type T7,
/*2*/
type T8,
};
```

# Diagnostics
Expand Down Expand Up @@ -206,6 +213,7 @@ invalid.ts:31:8 lint/style/useExportType FIXABLE ━━━━━━━━━
> 31 │ export {}
│ ^^
32 │
33 │ import { type T7, type T8 } from "./mod.ts";
i Using export type allows transpilers to safely drop exports of types without looking for their definition.
Expand All @@ -216,4 +224,36 @@ invalid.ts:31:8 lint/style/useExportType FIXABLE ━━━━━━━━━
```

```
invalid.ts:34:8 lint/style/useExportType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! All exports are only types and should thus use export type.
33 │ import { type T7, type T8 } from "./mod.ts";
> 34 │ export {
│ ^
> 35 │ /*1*/
> 36 │ type T7,
> 37 │ /*2*/
> 38 │ type T8,
> 39 │ };
│ ^^
i Using export type allows transpilers to safely drop exports of types without looking for their definition.
i Safe fix: Use a grouped export type.
32 32 │
33 33 │ import { type T7, type T8 } from "./mod.ts";
34 │ - export·{
34 │ + export·type·{
35 35 │ /*1*/
36 │ - ··type·T7,
36 │ + ··T7,
37 37 │ /*2*/
38 │ - ··type·T8,
38 │ + ··T8,
39 39 │ };
```

0 comments on commit ac22955

Please sign in to comment.