fix: avoid overlapping definition for T::Enum synthetic serialize() method

[chrapkowski-sg]

CU-2372 Removes the synthetic serialize() definition generation.

Motivation

When users click "Find References" on a T::Enum class name in Sourcegraph, the UI sometimes shows references to the serialize() method instead of references to the class itself.

This happens because scip-ruby emits two definitions at the exact same source location:

1. The class definition (e.g., Foo#)
2. The synthetic serialize() method (e.g., Foo#serialize().)

When the Sourcegraph frontend queries for the symbol at a position and finds multiple overlapping definitions, it may pick the wrong one, causing Find References to return incorrect results.

Now only definition for the class is provided:

Problem

The synthetic serialize() method definition was emitted at the T::Enum class
declaration location, causing overlapping definitions with the class itself.
This broke Find References in Sourcegraph UI, which would pick up serialize
references instead of class references.

Before

class Foo < T::Enum
#     ^^^ definition [..] Foo#
#     ^^^ definition [..] Foo#serialize().
#           ^ reference [..] T#
#              ^^^^ reference [..] Module#public().
#              ^^^^ reference [..] String#
#              ^^^^ reference [..] T#Enum#

After

class Foo < T::Enum
#     ^^^ definition [..] Foo#
#           ^ reference [..] T#
#              ^^^^ reference [..] T#Enum#

Test plan

See included automated tests.

[chrapkowski-sg]

This change is part of the following stack:

• Fix failing CI due to macos-13 deprecation #247
  • fix: avoid overlapping definition for T::Enum synthetic serialize() method #246 ◀

_{Change managed by git-spice.}

[jupblb]

Left two comments. Looks ok.

[jupblb]

What is this patch?

[chrapkowski-sg]

accidentally added

[jupblb]

Removing this is a serious deviation from the upstream:

scip-ruby/rewriter/TEnum.cc

Lines 241 to 257 in 0a7b175

	if (core::isa_type<core::ClassType>(serializeReturnType) && !serializeReturnType.isUntyped() &&
	!serializeReturnType.isBottom()) {
	auto serializeReturnTypeClass = core::cast_type_nonnull<core::ClassType>(serializeReturnType);
	ast::ExpressionPtr return_type_ast = ast::MK::Constant(klass->declLoc, serializeReturnTypeClass.symbol);
	auto sig = ast::MK::Sig0(klass->declLoc, std::move(return_type_ast));
	auto method = ast::MK::SyntheticMethod0(klass->loc, klass->declLoc, core::Names::serialize(),
	ast::MK::RaiseTypedUnimplemented(klass->declLoc));
	ast::Send::ARGS_store nargs;
	ast::Send::Flags flags;
	flags.isPrivateOk = true;
	auto visibility = ast::MK::Send(klass->declLoc, ast::MK::Self(klass->declLoc), core::Names::public_(),
	klass->declLoc, 0, std::move(nargs), flags);
	klass->rhs.emplace_back(std::move(visibility));
	klass->rhs.emplace_back(std::move(sig));
	klass->rhs.emplace_back(std::move(method));
	}

This is not bad but something worth noting. I hope this doesn't break anything.

[chrapkowski-sg]

Yes, we need to track this in case of rebases. We have this commit including PR and reasons why this has been removed