[Bug]: sort-prop-types's autofix doesn't move the semicolon, producing broken code, on single-line Props types

[tylerlaprade]

Is there an existing issue for this?

• I have searched the existing issues and my issue is unique
• My issue appears in the command-line and not only in the text editor

Description Overview

sort-prop-types, with checkTypes: true, breaks the code when run on single-line Props with autofix due to the semicolon staying at the end instead of moving.

The props were rearranged without the semicolon dividing them

eslint --fix

Expected Behavior

Semicolon be moved along with the moved prop when running autofix

type CustomProps = { onChange: (event: { target: { name: string; value: string } }) => void; name: string };

should become

type CustomProps = { name: string; onChange: (event: { target: { name: string; value: string } }) => void };

Instead, it is fixing to

type CustomProps = { name: string onChange: (event: { target: { name: string; value: string } }) => void; };

eslint-plugin-react version

v7.34.4

eslint version

v8.57.0

node version

v20.14.10

[tylerlaprade]

UPDATE: I found another case which produces an incorrect autofix, this one on a multi-line Props. It seems to be due to comments. Notice that the comments have been duplicated, three props have disappeared, and part of a typehint is on a line by itself.

Starting code:

type Props = {
  onClose: () => void;
  onSave?: () => void;
  initialContractInfo?: ContractInfo; // used to pre-populate the form just for our tests
  contractVersionTraceId?: TraceId; // used when editing an existing contract
  contractContainerId: TraceId;
  wizardStartIndex?: number;
  contractStatus?: BackendContractStatus;
  contractVersion?: BackendContractVersion;
};

Auto-fixed code:

type Props = {
  initialContractInfo?: ContractInfo; // used to pre-populate the form just for our tests
  // used to pre-populate the form just for our tests
  contractVersionTraceId?: TraceId; // used when editing an existing contract
  // used when editing an existing contract
  contractContainerId: TraceId;
  dContractVersion;
  onClose: () => void;
  onSave?: () => void;
};

Expected result:

type Props = {
  initialContractInfo?: ContractInfo; // used to pre-populate the form just for our tests
  contractVersionTraceId?: TraceId; // used when editing an existing contract
  contractContainerId: TraceId;
  wizardStartIndex?: number;
  contractStatus?: BackendContractStatus;
  contractVersion?: BackendContractVersion;
  onClose: () => void;
  onSave?: () => void;
};

[ljharb]

@tylerlaprade can you provide the component code these are associated with? it won't repro without a component.

[tylerlaprade]

Full code

type CustomProps = { onChange: (event: { target: { name: string; value: string } }) => void; name: string };

function PercentFormatCustom(props: CustomProps) {
  return (
    <div />
  );
}

type Props = {
  onClose: () => void;
  onSave?: () => void;
  initialContractInfo?: ContractInfo; // used to pre-populate the form just for our tests
  contractVersionTraceId?: TraceId; // used when editing an existing contract
  contractContainerId: TraceId;
  wizardStartIndex?: number;
  contractStatus?: BackendContractStatus;
  contractVersion?: BackendContractVersion;
};

function ContractVersionWizard(props: Props) {
  return <div />;
}

and the relevant rule config:

    "react/sort-prop-types": [
      "error",
      {
        "callbacksLast": true,
        "requiredFirst": true,
        "sortShapeProp": true,
        "noSortAlphabetically": true,
        "checkTypes": true
      }
    ]

[ljharb]

Thanks, that definitely reproduces.

I'm not sure how to fix it, tbh, but i can certainly disable the autofix for types.

[tylerlaprade]

The autofix has been quite helpful in other cases. I imagine this would just be an issue of including the semicolon and/or comment in the moved token.

[ljharb]

right - it's not super clear to me how to DO that tho :-/

cc @akulsr0 from #3615, maybe you have an idea?

[tylerlaprade]

@ljharb, in the description of #3784, @akulsr0 says it only partially fixes this issue. Should we leave it open for the other half of the issue?

[akulsr0]

I feel the second one can be a seperate issue, for better tracking.

[ljharb]

Yeah it'd be very helpful to file a separate issue for that. Thanks!