chore: remove unused code #1252

incendial · 2023-01-30T12:34:17Z

📜 Description

This PR removes some unused code.
#skip-changelog

💡 Motivation and Context

Hello! I'm one of the authors of DCM package and I'm currently testing new unused code check that also checks for methods, fields, etc.

I've noticed that you have an opened issue #949, so decide to submit this PR.

Here is full report for dart/ folder:

✔ Analysis is completed. Preparing the results: 18.3s

dart/lib/src/noop_sentry_client.dart:
    ⚠ unused constructor NoOpSentryClient
      at /Users/dimannich/Desktop/code/sentry-dart/dart/lib/src/noop_sentry_client.dart:15:3

dart/lib/src/protocol/max_body_size.dart:
    ⚠ unused method shouldAddBody
      at /Users/dimannich/Desktop/code/sentry-dart/dart/lib/src/protocol/max_body_size.dart:61:3
    ⚠ unused extension MaxResponseBodySizeX
      at /Users/dimannich/Desktop/code/sentry-dart/dart/lib/src/protocol/max_body_size.dart:60:1

dart/lib/src/sentry_client_stub.dart:
    ⚠ unused function createSentryClient
      at /Users/dimannich/Desktop/code/sentry-dart/dart/lib/src/sentry_client_stub.dart:8:1

dart/lib/src/sentry_item_type.dart:
    ⚠ unused field unknown
      at /Users/dimannich/Desktop/code/sentry-dart/dart/lib/src/sentry_item_type.dart:7:3

dart/lib/src/transport/encode.dart:
    ⚠ unused function compressBody
      at /Users/dimannich/Desktop/code/sentry-dart/dart/lib/src/transport/encode.dart:3:1

dart/lib/src/transport/noop_encode.dart:
    ⚠ unused function compressBody
      at /Users/dimannich/Desktop/code/sentry-dart/dart/lib/src/transport/noop_encode.dart:1:1

dart/test/mocks/mock_hub.dart:
    ⚠ unused method reset
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:27:3
    ⚠ unused field stackTrace
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:138:3
    ⚠ unused field hint
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:139:3
    ⚠ unused field throwable
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:145:3
    ⚠ unused field stackTrace
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:146:3
    ⚠ unused field hint
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:147:3
    ⚠ unused field message
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:157:3
    ⚠ unused field level
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:158:3
    ⚠ unused field template
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:159:3
    ⚠ unused field params
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:160:3
    ⚠ unused field hint
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:161:3
    ⚠ unused field hint
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_hub.dart:174:3

dart/test/mocks/mock_sentry_client.dart:
    ⚠ unused field hint
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_sentry_client.dart:98:3
    ⚠ unused field throwable
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_sentry_client.dart:109:3
    ⚠ unused field stackTrace
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_sentry_client.dart:110:3
    ⚠ unused field scope
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_sentry_client.dart:111:3
    ⚠ unused field hint
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_sentry_client.dart:112:3
    ⚠ unused field template
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_sentry_client.dart:125:3
    ⚠ unused field params
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_sentry_client.dart:126:3
    ⚠ unused field hint
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_sentry_client.dart:128:3
    ⚠ unused field envelope
      at /Users/dimannich/Desktop/code/sentry-dart/dart/test/mocks/mock_sentry_client.dart:141:3

✖ total unused code (classes, functions, variables, extensions, enums, mixins and type aliases) - 28

Here is full report for flutter/ folder:

✔ Analysis is completed. Preparing the results: 10.9s

flutter/lib/src/jvm/jvm_exception.dart:
    ⚠ unused method tryParse
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/lib/src/jvm/jvm_exception.dart:22:3

flutter/lib/src/jvm/jvm_frame.dart:
    ⚠ unused method tryParse
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/lib/src/jvm/jvm_frame.dart:24:3
    ⚠ unused getter unparsed
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/lib/src/jvm/jvm_frame.dart:67:3

flutter/test/integrations/flutter_error_integration_test.dart:
    ⚠ unused method getIntegration
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/test/integrations/flutter_error_integration_test.dart:219:3

flutter/test/integrations/load_contexts_integration_test.dart:
    ⚠ unused method getIntegration
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/test/integrations/load_contexts_integration_test.dart:46:3

flutter/test/integrations/native_sdk_integration_test.dart:
    ⚠ unused method getIntegration
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/test/integrations/native_sdk_integration_test.dart:101:3

flutter/test/integrations/widgets_flutter_binding_integration_test.dart:
    ⚠ unused method getIntegration
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/test/integrations/widgets_flutter_binding_integration_test.dart:53:3

flutter/test/mocks.dart:
    ⚠ unused constructor MockPlatform.fuchsia
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/test/mocks.dart:77:3

flutter/test/mocks.mocks.dart:
    ⚠ unused constructor MockSentryNative
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/test/mocks.mocks.dart:494:3
    ⚠ unused class MockSentryNative
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/test/mocks.mocks.dart:490:1

flutter/test/sentry_navigator_observer_test.dart:
    ⚠ unused method mockContext
      at /Users/dimannich/Desktop/code/sentry-dart/flutter/test/sentry_navigator_observer_test.dart:750:3

✖ total unused code (classes, functions, variables, extensions, enums, mixins and type aliases) - 11

Note: I've removed only some reported code, anything above is not touched. If you consider anything above worth removing, please let me know and I'll update the PR.

💚 How did you test it?

With dcm.

📝 Checklist

I reviewed submitted code
I added tests to verify changes
No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
I updated the docs if needed
All tests passing
No breaking changes

🔮 Next steps

Waiting for your feedback / comments 🙂.

marandaneto · 2023-01-30T12:48:37Z

@incendial thank you for the PR, it does make sense.
There are likely false positives in the report as well just because if the SDK itself doesn't use eg a ctor, it does not mean that users don't in their code, so before going ahead and removing everything, it has to be case by case but besides that, all in for PRs like this :)

codecov · 2023-01-30T12:49:26Z

Codecov Report

Base: 89.72% // Head: 76.45% // Decreases project coverage by -13.28% ⚠️

Coverage data is based on head (b0f9f1e) compared to base (e893df5).
Patch has no changes to coverable lines.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1252       +/-   ##
===========================================
- Coverage   89.72%   76.45%   -13.28%     
===========================================
  Files         155       11      -144     
  Lines        5034      327     -4707     
===========================================
- Hits         4517      250     -4267     
+ Misses        517       77      -440

Impacted Files	Coverage Δ
flutter/lib/src/jvm/jvm_exception.dart
dart/lib/src/protocol/sentry_request.dart
dart/lib/src/transport/data_category.dart
dart/lib/src/sentry_sampling_context.dart
flutter/lib/src/renderer/io_renderer.dart
...or/android_platform_exception_event_processor.dart
dart/lib/src/protocol/sentry_gpu.dart
...art/lib/src/http_client/failed_request_client.dart
dart/lib/src/client_reports/client_report.dart
dart/lib/src/protocol/sentry_baggage_header.dart
... and 134 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

incendial · 2023-01-30T13:02:57Z

There are likely false positives in the report as well just because if the SDK itself doesn't use eg a ctor, it does not mean that users don't in their code, so before going ahead and removing everything, it has to be case by case but besides that, all in for PRs like this :)

The command should not report anything that is exported directly or indirectly, so if you see anything publicly available - please let me know.

incendial · 2023-01-30T13:06:10Z

so before going ahead and removing everything, it has to be case by case

Can someone from the team take a closer look?

marandaneto · 2023-01-30T13:16:22Z

so before going ahead and removing everything, it has to be case by case

Can someone from the team take a closer look?

The current removal makes sense the way how it is, I'd approve if it's only that.
What I meant is about some other warnings.

Eg

flutter/lib/src/jvm/jvm_exception.dart:
⚠ unused method tryParse
at /Users/dimannich/Desktop/code/sentry-dart/flutter/lib/src/jvm/jvm_exception.dart:22:3

This one makes sense and could be removed, I dont expect that to be called externally.

flutter/test/mocks.dart:
⚠ unused constructor MockPlatform.fuchsia
at /Users/dimannich/Desktop/code/sentry-dart/flutter/test/mocks.dart:77:3

Will likely be used in the future once 1st class Fuchsia support lands.

marandaneto

Linter is complaining besides that LGTM.

incendial · 2023-01-30T13:37:57Z

Linter is complaining besides that LGTM.

I'll fix that and remove the mentioned tryParse 👍 .

The current removal makes sense the way how it is, I'd approve if it's only that.

Sure 👍 , but now I'm curious about NoOpSentryClient - it's not referenced and not exported (or I don't see it), how you / your users use it? 🙂

marandaneto · 2023-01-30T13:40:43Z

Linter is complaining besides that LGTM.

I'll fix that and remove the mentioned tryParse 👍 .

The current removal makes sense the way how it is, I'd approve if it's only that.

Sure 👍 , but now I'm curious about NoOpSentryClient - it's not referenced and not exported (or I don't see it), how you / your users use it? 🙂

That was before the sound null safety, now that Dart is fully sound null safety on v3, we can likely refactor all of that, but I'd rather do that on the next major version, some public APIs would return nullable values most likely, good observation.

incendial · 2023-01-30T13:47:49Z

Hm, why are you mentioning nullsafety, when the report is about unused code?

It's not about nullable parameters as you commented here #949 (comment). This one is another command.

marandaneto · 2023-01-30T13:59:35Z

NoOpSentryClient

I was just answering your question about NoOpSentryClient, why it even exists in the source code, and it's historical reasons, that's why its not used and detected by the analyzer.

incendial · 2023-01-30T14:01:12Z

Ah, I see, thank you! I've updated the PR.

marandaneto

Thanks! LGTM

chore: remove unused code

31407e7

incendial requested review from marandaneto, brustolin and krystofwoldrich as code owners January 30, 2023 12:34

marandaneto approved these changes Jan 30, 2023

View reviewed changes

incendial added 2 commits January 30, 2023 17:51

chore: remove unused import

0046a41

chore: remove JvmException tryParse

763aed1

marandaneto approved these changes Jan 30, 2023

View reviewed changes

Merge branch 'main' into remove-unused-code

b0f9f1e

marandaneto enabled auto-merge (squash) February 6, 2023 13:27

marandaneto merged commit 3637a22 into getsentry:main Feb 6, 2023

incendial deleted the remove-unused-code branch February 6, 2023 14:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: remove unused code #1252

chore: remove unused code #1252

incendial commented Jan 30, 2023 •

edited by marandaneto

Loading

marandaneto commented Jan 30, 2023

codecov bot commented Jan 30, 2023 •

edited

Loading

incendial commented Jan 30, 2023

incendial commented Jan 30, 2023

marandaneto commented Jan 30, 2023

marandaneto left a comment

incendial commented Jan 30, 2023

marandaneto commented Jan 30, 2023

incendial commented Jan 30, 2023 •

edited

Loading

marandaneto commented Jan 30, 2023

incendial commented Jan 30, 2023

marandaneto left a comment

chore: remove unused code #1252

chore: remove unused code #1252

Conversation

incendial commented Jan 30, 2023 • edited by marandaneto Loading

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

marandaneto commented Jan 30, 2023

codecov bot commented Jan 30, 2023 • edited Loading

Codecov Report

incendial commented Jan 30, 2023

incendial commented Jan 30, 2023

marandaneto commented Jan 30, 2023

marandaneto left a comment

Choose a reason for hiding this comment

incendial commented Jan 30, 2023

marandaneto commented Jan 30, 2023

incendial commented Jan 30, 2023 • edited Loading

marandaneto commented Jan 30, 2023

incendial commented Jan 30, 2023

marandaneto left a comment

Choose a reason for hiding this comment

incendial commented Jan 30, 2023 •

edited by marandaneto

Loading

codecov bot commented Jan 30, 2023 •

edited

Loading

incendial commented Jan 30, 2023 •

edited

Loading