Skip to content

RoslynCodeTaskFactory: Log MSB3753 when task class does not implement ITask#13517

Merged
jankratochvilcz merged 6 commits into
mainfrom
jankratochvilcz/roslyncodetaskfactory-missing-itask-check
Apr 10, 2026
Merged

RoslynCodeTaskFactory: Log MSB3753 when task class does not implement ITask#13517
jankratochvilcz merged 6 commits into
mainfrom
jankratochvilcz/roslyncodetaskfactory-missing-itask-check

Conversation

@jankratochvilcz
Copy link
Copy Markdown
Contributor

@jankratochvilcz jankratochvilcz commented Apr 9, 2026

Summary

RoslynCodeTaskFactory.CreateTask() silently returned null when the inline task class did not implement ITask, causing the engine to emit the generic MSB4060 error. CodeTaskFactory already checks for this and emits the more specific MSB3753 (CodeTaskFactory.NeedsITaskInterface).

Changes

  • RoslynCodeTaskFactory.cs: Added null check after Activator.CreateInstance(TaskType) as ITask — logs MSB3753 via the existing _log instance (set during Initialize), matching CodeTaskFactory behavior.
  • RoslynCodeTaskFactory_Tests.cs: Added ClassDoesNotInheritFromITask test covering both in-proc and out-of-proc (MSBUILDFORCEINLINETASKFACTORIESOUTOFPROC) paths.

Context

I noticed this when trying to migrate usage of CodeTaskFactory to RoslynCodeTaskFactory and checking for coverage parity between these two factories as part of that.

The CodeTaskFactory equivalent test (CodeTaskFactoryTests.cs) already validates this behavior. This was an oversight in RoslynCodeTaskFactory — the two factories should produce the same diagnostic for this user error.

Copilot AI review requested due to automatic review settings April 9, 2026 14:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns RoslynCodeTaskFactory diagnostics with CodeTaskFactory by emitting MSB3753 when an inline task class doesn’t implement ITask, instead of failing later with a generic MSB4060.

Changes:

  • Add a null-check in RoslynCodeTaskFactory.CreateTask() and log CodeTaskFactory.NeedsITaskInterface (MSB3753) when the created instance can’t be cast to ITask.
  • Add an end-to-end unit test validating the MSB3753 behavior for both in-proc and forced out-of-proc inline task factory paths.
  • Add a new regression test covering successful execution when the temp directory path does not exist.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs Adds MSB3753 logging when the compiled inline task type does not implement ITask.
src/Tasks.UnitTests/RoslynCodeTaskFactory_Tests.cs Adds coverage for the new MSB3753 behavior and an additional temp-directory regression scenario.

Comment thread src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs Outdated
Comment thread src/Tasks.UnitTests/RoslynCodeTaskFactory_Tests.cs
Comment thread src/Tasks.UnitTests/RoslynCodeTaskFactory_Tests.cs Outdated
Comment thread src/Tasks.UnitTests/RoslynCodeTaskFactory_Tests.cs Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Awareness — LGTM

No security regression. Activator.CreateInstance(TaskType) was already the existing instantiation path; the only addition is a null-guard with error logging. No changes to type loading policy, file operations, or path handling.

Note

🔒 Integrity filter blocked 1 item

The following item were blocked because they don't meet the GitHub integrity level.

  • #13517 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Expert Code Review (on open) for issue #13517 · ● 25M

Comment thread src/Tasks.UnitTests/RoslynCodeTaskFactory_Tests.cs Outdated
Comment thread src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs Outdated
Comment thread src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs Outdated
Comment thread src/Tasks.UnitTests/RoslynCodeTaskFactory_Tests.cs
Comment thread src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs Outdated
@jankratochvilcz jankratochvilcz enabled auto-merge (squash) April 10, 2026 12:28
@jankratochvilcz jankratochvilcz merged commit e2ae904 into main Apr 10, 2026
10 checks passed
@jankratochvilcz jankratochvilcz deleted the jankratochvilcz/roslyncodetaskfactory-missing-itask-check branch April 10, 2026 13:21
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.

3 participants