ronantakizawa/codereview-bench huggingface dataset:
Repository: rollup/rollup
File: build-plugins/load-cli-help.ts (TypeScript)

Code before:
import { readFile } from 'node:fs/promises';
import type { Plugin } from 'rollup';

export default function loadCliHelp(): Plugin {
	return {
		async load(id) {
			if (id.endsWith('help.md')) {
				const [rawHelpFile, packageFile] = await Promise.all([
					readFile(id, 'utf8'),
					readFile(new URL('../package.json', import.meta.url), 'utf8')
				]);
				const finalHelpFile = rawHelpFile
					.replaceAll(/^\/\/[^\n]*\n?/gm, '')
					.replace('__VERSION__', JSON.parse(packageFile).version);
				return `export default ${JSON.stringify(finalHelpFile)};`;
			}
		},
		name: 'load-cli-help'
	};
}


Review comment (suggestion):
The plugin should return `null` or `undefined` when the condition is not met to allow other plugins to handle the file. Currently, if a file doesn't end with 'help.md', the `load` hook returns `undefined` implicitly, which is correct but could be more explicit for clarity. Adding `return null;` after the `if` block would make the intent clearer.
```suggestion
			}
			return null;
```

Code after:
import { readFile } from 'node:fs/promises';
import type { Plugin } from 'rollup';

export default function loadCliHelp(): Plugin {
	return {
		async load(id) {
			if (id.endsWith('help.md')) {
				const [rawHelpFile, packageFile] = await Promise.all([
					readFile(id, 'utf8'),
					readFile(new URL('../package.json', import.meta.url), 'utf8')
				]);
				const finalHelpFile = rawHelpFile
					.replaceAll(/^\/\/\s*#(end)?region[^\n]*\n/gm, '')
					.replace('__VERSION__', JSON.parse(packageFile).version);
				return `export default ${JSON.stringify(finalHelpFile)};`;
			}
			return null;
		},
		name: 'load-cli-help'
	};
}
Repository: rollup/rollup
File: src/ast/nodes/ExportAllDeclaration.ts (TypeScript)

Code before:
import type MagicString from 'magic-string';
import type { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
import type Identifier from './Identifier';
import type ImportAttribute from './ImportAttribute';
import type Literal from './Literal';
import type * as NodeType from './NodeType';
import { doNotDeoptimize, NodeBase, onlyIncludeSelfNoDeoptimize } from './shared/Node';

export default class ExportAllDeclaration extends NodeBase {
	attributes!: ImportAttribute[];
	exported!: Identifier | Literal<string> | null;
	needsBoundaries!: true;
	source!: Literal<string>;
	type!: NodeType.tExportAllDeclaration;

	hasEffects(): boolean {
		return false;
	}

	initialise(): void {
		super.initialise();
		this.scope.context.addExport(this);
	}

	render(code: MagicString, _options: RenderOptions, nodeRenderOptions?: NodeRenderOptions): void {
		code.remove(nodeRenderOptions!.start!, nodeRenderOptions!.end!);
	}
}

ExportAllDeclaration.prototype.needsBoundaries = true;
ExportAllDeclaration.prototype.includeNode = onlyIncludeSelfNoDeoptimize;
ExportAllDeclaration.prototype.applyDeoptimizations = doNotDeoptimize;


Review comment (suggestion):
`ExportAllDeclaration.prototype.needsBoundaries = true` is set later in this file. Declaring `needsBoundaries!: true` emits an instance field initialized to `undefined`, which shadows the prototype flag and makes `needsBoundaries` falsy at runtime. This should remain `declare needsBoundaries: true` or be initialized to `true` as a real field.
```suggestion
	declare needsBoundaries: true;
```

Code after:
import type MagicString from 'magic-string';
import type { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
import type Identifier from './Identifier';
import type ImportAttribute from './ImportAttribute';
import type Literal from './Literal';
import type * as NodeType from './NodeType';
import { doNotDeoptimize, NodeBase, onlyIncludeSelfNoDeoptimize } from './shared/Node';

export default class ExportAllDeclaration extends NodeBase {
	declare attributes: ImportAttribute[];
	declare exported: Identifier | Literal<string> | null;
	declare needsBoundaries: true;
	declare source: Literal<string>;
	declare type: NodeType.tExportAllDeclaration;

	hasEffects(): boolean {
		return false;
	}

	initialise(): void {
		super.initialise();
		this.scope.context.addExport(this);
	}

	render(code: MagicString, _options: RenderOptions, nodeRenderOptions?: NodeRenderOptions): void {
		code.remove(nodeRenderOptions!.start!, nodeRenderOptions!.end!);
	}
}

ExportAllDeclaration.prototype.needsBoundaries = true;
ExportAllDeclaration.prototype.includeNode = onlyIncludeSelfNoDeoptimize;
ExportAllDeclaration.prototype.applyDeoptimizations = doNotDeoptimize;
Repository: rust-lang/rust
File: compiler/rustc_attr_parsing/src/attributes/cfg_select.rs (Rust)

Code before:

            p.expect(exp!(FatArrow)).map_err(|e| e.emit())?;

            let tts = p.parse_delimited_token_tree().map_err(|e| e.emit())?;
            let span = cfg_span.to(p.token.span);

            match branches.wildcard {
                None => branches.reachable.push((cfg, tts, span)),
                Some(_) => branches.unreachable.push((CfgSelectPredicate::Cfg(cfg), tts, span)),
            }
        }
    }

    if let Some((underscore, _, _)) = branches.wildcard
        && features.map_or(false, |f| f.enabled(rustc_span::sym::cfg_select))
    {
        for (predicate, _, _) in &branches.unreachable {
            let span = predicate.span();
            p.psess.buffer_lint(
                UNREACHABLE_CFG_SELECT_PREDICATES,
                span,
                lint_node_id,
                BuiltinLintDiag::UnreachableCfg { span, wildcard_span: underscore.span },
            );
        }
    }

    Ok(branches)
}


Review comment (suggestion):
This doesn't seem to warn, unless there is a wildcard, why so? Does it not warn in case of

```rust
cfg_select! {
    unix => true,
    not(unix) => false,
    windows => false,
}
```

Code after:
            match branches.wildcard {
                None => branches.wildcard = Some((underscore, tts, span)),
                Some(_) => {
                    branches.unreachable.push((CfgSelectPredicate::Wildcard(underscore), tts, span))
                }
            }
        } else {
            let meta = MetaItemOrLitParser::parse_single(
                p,
                ShouldEmit::ErrorsAndLints { recovery: Recovery::Allowed },
            )
            .map_err(|diag| diag.emit())?;
            let cfg_span = meta.span();
            let cfg = AttributeParser::parse_single_args(
                sess,
                cfg_span,
                cfg_span,
                AttrStyle::Inner,
                AttrPath { segments: vec![sym::cfg_select].into_boxed_slice(), span: cfg_span },
                None,
                ParsedDescription::Macro,
                cfg_span,
                lint_node_id,
                // Doesn't matter what the target actually is here.
                Target::Crate,
                features,
                ShouldEmit::ErrorsAndLints { recovery: Recovery::Allowed },
                &meta,
                parse_cfg_entry,
                &AttributeTemplate::default(),
            )?;

            p.expect(exp!(FatArrow)).map_err(|e| e.emit())?;

            let tts = p.parse_delimited_token_tree().map_err(|e| e.emit())?;
            let span = cfg_span.to(p.token.span);

            match branches.wildcard {
                None => branches.reachable.push((cfg, tts, span)),
                Some(_) => branches.unreachable.push((CfgSelectPredicate::Cfg(cfg), tts, span)),
            }
        }
    }

    if let Some(features) = features
        && features.enabled(sym::cfg_select)
    {
        let it = branches
            .reachable
            .iter()
            .map(|(entry, _, _)| CfgSelectPredicate::Cfg(entry.clone()))
Repository: rust-lang/rust
File: compiler/rustc_lint/src/builtin.rs (Rust)

Code before:
    /// Unfortunately, it was not explicitly marked as unstable and some
    /// people have now mistakenly assumed they had to implement this method.
    ///
    /// As the method is never called by the standard library, you can safely
    /// remove any implementations of the method and just write `impl Eq for Foo {}`.
    ///
    /// This is a [future-incompatible] lint to transition this to a hard
    /// error in the future. See [issue #152336] for more details.
    ///
    /// [issue #152336]: https://github.com/rust-lang/rust/issues/152336
    pub INTERNAL_EQ_TRAIT_METHOD_IMPLS,
    Warn,
    "manual implementation of the internal `Eq::assert_receiver_is_total_eq` method",
    @future_incompatible = FutureIncompatibleInfo {
        reason: fcw!(FutureReleaseError #152336),
        report_in_deps: false,
    };
}

declare_lint_pass!(InternalEqTraitMethodImpls => [INTERNAL_EQ_TRAIT_METHOD_IMPLS]);

impl<'tcx> LateLintPass<'tcx> for InternalEqTraitMethodImpls {
    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx rustc_hir::ImplItem<'tcx>) {
        if let ImplItemImplKind::Trait { defaultness: _, trait_item_def_id: Ok(trait_item_def_id) } =
            item.impl_kind
            && item.ident.name == sym::assert_receiver_is_total_eq
            && cx.tcx.is_diagnostic_item(sym::assert_receiver_is_total_eq, trait_item_def_id)
        {
            cx.emit_span_lint(
                INTERNAL_EQ_TRAIT_METHOD_IMPLS,
                item.span,
                EqInternalMethodImplemented,
            );
        }
    }
}


Review comment (performance):
I don't think this check is necessary if you're checking that it's the diagnostic item right after?
```suggestion
```

Or was it for performance? If so, then that seems like something we'd want more generally.

Code after:
    /// remove any implementations of the method and just write `impl Eq for Foo {}`.
    ///
    /// This is a [future-incompatible] lint to transition this to a hard
    /// error in the future. See [issue #152336] for more details.
    ///
    /// [issue #152336]: https://github.com/rust-lang/rust/issues/152336
    pub INTERNAL_EQ_TRAIT_METHOD_IMPLS,
    Warn,
    "manual implementation of the internal `Eq::assert_receiver_is_total_eq` method",
    @future_incompatible = FutureIncompatibleInfo {
        reason: fcw!(FutureReleaseError #152336),
        report_in_deps: false,
    };
}

declare_lint_pass!(InternalEqTraitMethodImpls => [INTERNAL_EQ_TRAIT_METHOD_IMPLS]);

impl<'tcx> LateLintPass<'tcx> for InternalEqTraitMethodImpls {
    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx rustc_hir::ImplItem<'tcx>) {
        if let ImplItemImplKind::Trait { defaultness: _, trait_item_def_id: Ok(trait_item_def_id) } =
            item.impl_kind
            && cx.tcx.is_diagnostic_item(sym::assert_receiver_is_total_eq, trait_item_def_id)
        {
            cx.emit_span_lint(
                INTERNAL_EQ_TRAIT_METHOD_IMPLS,
                item.span,
                EqInternalMethodImplemented,
            );
        }
    }
}
Repository: rust-lang/rust
File: compiler/rustc_lint/src/lints.rs (Rust)

Code before:
pub(crate) struct MissingOptionsForOnUnimplementedAttr;

#[derive(LintDiagnostic)]
#[diag("missing options for `on_const` attribute")]
#[help("at least one of the `message`, `note` and `label` options are expected")]
pub(crate) struct MissingOptionsForOnConstAttr;

#[derive(LintDiagnostic)]
#[diag("malformed `on_unimplemented` attribute")]
#[help("only `message`, `note` and `label` are allowed as options")]
pub(crate) struct MalformedOnUnimplementedAttrLint {
    #[label("invalid option found here")]
    pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag("malformed `on_const` attribute")]
#[help("only `message`, `note` and `label` are allowed as options")]
pub(crate) struct MalformedOnConstAttrLint {
    #[label("invalid option found here")]
    pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag("`Eq::assert_receiver_is_total_eq` should never be implemented by hand")]
#[note("this method is only used to add checks to the `Eq` derive macro")]
pub(crate) struct EqInternalMethodImplemented;


Review comment (suggestion):
```suggestion
#[note("this method was used to add checks to the `Eq` derive macro")]
```

Code after:
pub(crate) struct MissingOptionsForOnUnimplementedAttr;

#[derive(LintDiagnostic)]
#[diag("missing options for `on_const` attribute")]
#[help("at least one of the `message`, `note` and `label` options are expected")]
pub(crate) struct MissingOptionsForOnConstAttr;

#[derive(LintDiagnostic)]
#[diag("malformed `on_unimplemented` attribute")]
#[help("only `message`, `note` and `label` are allowed as options")]
pub(crate) struct MalformedOnUnimplementedAttrLint {
    #[label("invalid option found here")]
    pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag("malformed `on_const` attribute")]
#[help("only `message`, `note` and `label` are allowed as options")]
pub(crate) struct MalformedOnConstAttrLint {
    #[label("invalid option found here")]
    pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag("`Eq::assert_receiver_is_total_eq` should never be implemented by hand")]
#[note("this method was used to add checks to the `Eq` derive macro")]
pub(crate) struct EqInternalMethodImplemented;
Repository: rust-lang/rust
File: compiler/rustc_driver_impl/src/highlighter.rs (Rust)

Code before:
//! This module prvides a syntax highlighter for Rust code.
//! It is used by the `rustc --explain` command.
//!
//! The syntax highlighter uses `rustc_parse`'s `source_str_to_stream`
//! function to parse the Rust code into a stream of tokens.
//! The highlighter then highlights the tokens in the stream,
//! and writes the highlighted output to the buffer.
use std::io::{self, Write};

use anstyle::{AnsiColor, Color, Effects, Style};
use rustc_lexer::{LiteralKind, strip_shebang, tokenize};

/// Highlight a Rust code string and write the highlighted
/// output to the buffer. It serves as a wrapper around
/// `Highlighter::highlight_rustc_lexer`. It is passed to
/// `write_anstream_buf_with_formatter` in the `lib.rs` file.
pub fn highlight(code: &str, buf: &mut Vec<u8>) -> io::Result<()> {
    let mut highlighter = Highlighter::default();
    highlighter.highlight_rustc_lexer(code, buf)
}

/// A syntax highlighter for Rust code
/// It is used by the `rustc --explain` command.
#[derive(Default)]
pub struct Highlighter {
    /// Used to track if the previous token was a token

Review comment (suggestion):
```suggestion
//! This module provides a syntax highlighter for Rust code.
```

Code after:
//! This module provides a syntax highlighter for Rust code.
//! It is used by the `rustc --explain` command.
//!
//! The syntax highlighter uses `rustc_lexer`'s `tokenize`
//! function to parse the Rust code into a `Vec` of tokens.
//! The highlighter then highlights the tokens in the `Vec`,
//! and writes the highlighted output to the buffer.
use std::io::{self, Write};

use anstyle::{AnsiColor, Color, Effects, Style};
use rustc_lexer::{LiteralKind, strip_shebang, tokenize};

const PRIMITIVE_TYPES: &'static [&str] = &[
    "i8", "i16", "i32", "i64", "i128", "isize", // signed integers
    "u8", "u16", "u32", "u64", "u128", "usize", // unsigned integers
    "f32", "f64", // floating point
    "char", "bool", // others
];

const KEYWORDS: &'static [&str] = &[
    "static", "struct", "super", "trait", "true", "type", "unsafe", "use", "where", "while", "as",
    "async", "await", "break", "const", "continue", "crate", "dyn", "else", "enum", "extern",
    "false", "fn", "for", "if", "impl", "in", "let", "loop", "match", "mod", "move", "mut", "pub",
    "ref",
];
Repository: rust-lang/rust
File: tests/rustdoc-html/display-hidden-items.rs (Rust)

Code before:
// Test to ensure that the `--document-hidden-items` option is working as expected.
//@ compile-flags: -Z unstable-options --document-hidden-items
// ignore-tidy-linelength

#![crate_name = "foo"]

//@ has 'foo/index.html'
//@ has - '//dt/span[@title="Hidden item"]' '👻'

//@ has - '//*[@id="reexport.hidden_reexport"]/code/*[@class="code-attribute"]' '#[doc(hidden)]'
//@ has - '//*[@id="reexport.hidden_reexport"]/code' 'pub use hidden::inside_hidden as hidden_reexport;'
#[doc(hidden)]
pub use hidden::inside_hidden as hidden_reexport;

//@ has - '//dt/a[@class="trait"]' 'TraitHidden'
//@ has 'foo/trait.TraitHidden.html'
//@ has 'foo/trait.TraitHidden.html' '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[doc(hidden)]'
//@ has 'foo/trait.TraitHidden.html' '//*[@class="rust item-decl"]/code' 'pub trait TraitHidden'
#[doc(hidden)]
pub trait TraitHidden {}

//@ has 'foo/index.html' '//dt/a[@class="trait"]' 'Trait'
pub trait Trait {
    //@ has 'foo/trait.Trait.html'
    //@ has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]/*[@class="code-attribute"]' '#[doc(hidden)]'
    #[doc(hidden)]
    const BAR: u32 = 0;

    //@ has - '//*[@id="method.foo"]/*[@class="code-header"]/*[@class="code-attribute"]' '#[doc(hidden)]'
    //@ has - '//*[@id="method.foo"]/*[@class="code-header"]' 'fn foo()'
    #[doc(hidden)]
    fn foo() {}
}

//@ has 'foo/index.html' '//dt/a[@class="struct"]' 'Struct'

Review comment (suggestion):
Ok, so for re-exports, I think we should instead use the ghost emoju like for the other items on the module page.

Code after:
// Test to ensure that the `--document-hidden-items` option is working as expected.
//@ compile-flags: -Z unstable-options --document-hidden-items
// ignore-tidy-linelength

#![crate_name = "foo"]

//@ has 'foo/index.html'

//@ matches - '//dt[code]' 'pub extern crate .*hidden_core;.*👻'
//@ has - '//dt/code' 'pub extern crate core as hidden_core;'
#[doc(hidden)]
pub extern crate core as hidden_core;

//@ has - '//*[@id="reexport.hidden_reexport"]/span[@title="Hidden item"]' '👻'
//@ has - '//*[@id="reexport.hidden_reexport"]/code' 'pub use hidden::inside_hidden as hidden_reexport;'
#[doc(hidden)]
pub use hidden::inside_hidden as hidden_reexport;

//@ has - '//dt/a[@class="trait"]' 'TraitHidden'
//@ has 'foo/trait.TraitHidden.html'
//@ has 'foo/trait.TraitHidden.html' '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[doc(hidden)]'
//@ has 'foo/trait.TraitHidden.html' '//*[@class="rust item-decl"]/code' 'pub trait TraitHidden'
#[doc(hidden)]
pub trait TraitHidden {}

//@ has 'foo/index.html' '//dt/a[@class="trait"]' 'Trait'
pub trait Trait {
    //@ has 'foo/trait.Trait.html'
    //@ has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]/*[@class="code-attribute"]' '#[doc(hidden)]'
    #[doc(hidden)]
    const BAR: u32 = 0;

    //@ has - '//*[@id="method.foo"]/*[@class="code-header"]/*[@class="code-attribute"]' '#[doc(hidden)]'
    //@ has - '//*[@id="method.foo"]/*[@class="code-header"]' 'fn foo()'
    #[doc(hidden)]
Repository: rust-lang/rust
File: compiler/rustc_target/src/spec/targets/s390x_unknown_none_softfloat.rs (Rust)

Code before:
    Abi, Arch, Cc, LinkerFlavor, Lld, PanicStrategy, RelocModel, RustcAbi, SanitizerSet,
    StackProbeType, Target, TargetMetadata, TargetOptions,
};

pub(crate) fn target() -> Target {
    let opts = TargetOptions {
        abi: Abi::SoftFloat,
        cpu: "z10".into(),
        endian: Endian::Big,
        features: "+soft-float,-vector".into(),
        linker_flavor: LinkerFlavor::Gnu(Cc::No, Lld::Yes),
        linker: Some("rust-lld".into()),
        max_atomic_width: Some(128),
        min_global_align: Some(Align::from_bits(16).unwrap()),
        panic_strategy: PanicStrategy::Abort,
        relocation_model: RelocModel::Static,
        rustc_abi: Some(RustcAbi::Softfloat),
        stack_probes: StackProbeType::Inline,
        supported_sanitizers: SanitizerSet::KERNELADDRESS,
        ..Default::default()
    };

    Target {
        llvm_target: "s390x-unknown-linux-gnu".into(),
        metadata: TargetMetadata {
            description: Some("S390x Linux ".into()),
            host_tools: Some(false),
            std: Some(false),
            tier: Some(2),
        },
        arch: Arch::S390x,
        data_layout: "E-S64-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64".into(),
        options: opts,
        pointer_width: 64,
    }
}


Review comment (suggestion):
hm, does this need that space?
```suggestion
            description: Some("S390x Linux".into()),
```

Code after:
    Abi, Arch, Cc, LinkerFlavor, Lld, PanicStrategy, RelocModel, RustcAbi, SanitizerSet,
    StackProbeType, Target, TargetMetadata, TargetOptions,
};

pub(crate) fn target() -> Target {
    let opts = TargetOptions {
        abi: Abi::SoftFloat,
        cpu: "z10".into(),
        endian: Endian::Big,
        features: "+soft-float,-vector".into(),
        linker_flavor: LinkerFlavor::Gnu(Cc::No, Lld::Yes),
        linker: Some("rust-lld".into()),
        max_atomic_width: Some(128),
        min_global_align: Some(Align::from_bits(16).unwrap()),
        panic_strategy: PanicStrategy::Abort,
        relocation_model: RelocModel::Static,
        rustc_abi: Some(RustcAbi::Softfloat),
        stack_probes: StackProbeType::Inline,
        supported_sanitizers: SanitizerSet::KERNELADDRESS,
        ..Default::default()
    };

    Target {
        llvm_target: "s390x-unknown-linux-gnu".into(),
        metadata: TargetMetadata {
            description: Some("S390x Linux".into()),
            host_tools: Some(false),
            std: Some(false),
            tier: Some(2),
        },
        arch: Arch::S390x,
        data_layout: "E-S64-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64".into(),
        options: opts,
        pointer_width: 64,
    }
}