Skip to content

Conversation

@hadley
Copy link
Member

@hadley hadley commented Dec 31, 2025

Fixes #2302

@claude
Copy link

claude bot commented Dec 31, 2025

Claude finished @hadley's task —— View job


PR Review: Enhance OO mocking

Reviewed the OO mocking enhancements. Found one potential issue:

Issue in remove_s3_method() (R/mock-oo.R:66-69):

if (env_has(s3_envir, ".__S3MethodsTable__.")) {
  table <- env_get(s3_envir, ".__S3MethodsTable__.")
  env_unbind(table, paste0(generic, ".", class))
}

The function should check if the method exists in the table before attempting to unbind it. Calling env_unbind() on a non-existent key could cause issues.

Suggested fix:

if (env_has(s3_envir, ".__S3MethodsTable__.")) {
  table <- env_get(s3_envir, ".__S3MethodsTable__.")
  method_name <- paste0(generic, ".", class)
  if (env_has(table, method_name)) {
    env_unbind(table, method_name)
  }
}

Otherwise, the changes look solid with good test coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

local_mocked_s4_method() doesn't work for generics from other packages

2 participants