Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type Error, Random buffer, and Core #4327

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions Steepfile
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ target :datadog do
ignore 'lib/datadog/appsec/configuration/settings.rb'
ignore 'lib/datadog/appsec/contrib/'
ignore 'lib/datadog/appsec/monitor/gateway/watcher.rb'
ignore 'lib/datadog/core.rb'
ignore 'lib/datadog/core/buffer/random.rb'
ignore 'lib/datadog/core/buffer/thread_safe.rb'
ignore 'lib/datadog/core/configuration.rb'
ignore 'lib/datadog/core/configuration/base.rb'
Expand All @@ -67,7 +65,6 @@ target :datadog do
ignore 'lib/datadog/core/environment/socket.rb'
ignore 'lib/datadog/core/environment/variable_helpers.rb'
ignore 'lib/datadog/core/environment/vm_cache.rb'
ignore 'lib/datadog/core/error.rb'
ignore 'lib/datadog/core/metrics/client.rb'
ignore 'lib/datadog/core/metrics/helpers.rb'
ignore 'lib/datadog/core/metrics/metric.rb'
Expand Down
18 changes: 10 additions & 8 deletions lib/datadog/core/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@ class Error
class << self
def build_from(value)
case value
when Error then value
when Array then new(*value)
# A Ruby {Exception} is the most common parameter type.
when Exception then new(value.class, value.message, full_backtrace(value))
when ContainsMessage then new(value.class, value.message)
# steep:ignore:start
# Steep doesn't like an array with up to 3 elements to be passed here: it thinks the array is unbounded.
when Array then new(*value)
Copy link
Contributor

Choose a reason for hiding this comment

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

Code Quality Violation

Suggested change
when Array then new(*value)
when Arraynew(*value)
Don't use `then` for multi-line if, unless, when, or in statements (...read more)

The then keyword is not necessary in multi-line if/unless/when/in statements in Ruby. When used in multi-line statements, it can make the code harder to read and understand. This is because then is typically associated with single-line conditional statements in Ruby, and its use in multi-line statements can be confusing.

Maintaining readability and clarity in your code is crucial for effective collaboration and debugging. It becomes even more important in larger codebases, where complex logic can become difficult to follow if not written clearly.

To avoid this issue, omit the then keyword in your multi-line if/unless/when/in statements. For single-line if/unless/when/in statements, using then is acceptable and can help improve readability. This practice keeps your code clean and easy to understand, following the principles of good coding practices.

View in Datadog  Leave us feedback  Documentation

# Steep can follow the logic inside the lambda, thus it doesn't know `value` responds to `:message`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Steep can follow the logic inside the lambda, thus it doesn't know `value` responds to `:message`.
# Steep can not follow the logic inside the lambda, thus it doesn't know `value` responds to `:message`.

when ->(v) { v.respond_to?(:message) } then new(value.class, value.message)
Copy link
Contributor

Choose a reason for hiding this comment

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

Code Quality Violation

Suggested change
when ->(v) { v.respond_to?(:message) } then new(value.class, value.message)
when ->(v) { v.respond_to?(:message) }new(value.class, value.message)
Don't use `then` for multi-line if, unless, when, or in statements (...read more)

The then keyword is not necessary in multi-line if/unless/when/in statements in Ruby. When used in multi-line statements, it can make the code harder to read and understand. This is because then is typically associated with single-line conditional statements in Ruby, and its use in multi-line statements can be confusing.

Maintaining readability and clarity in your code is crucial for effective collaboration and debugging. It becomes even more important in larger codebases, where complex logic can become difficult to follow if not written clearly.

To avoid this issue, omit the then keyword in your multi-line if/unless/when/in statements. For single-line if/unless/when/in statements, using then is acceptable and can help improve readability. This practice keeps your code clean and easy to understand, following the principles of good coding practices.

View in Datadog  Leave us feedback  Documentation

# steep:ignore:end
when String then new(nil, value)
Copy link
Member

Choose a reason for hiding this comment

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

We have Error.new lower, does it makes sense to have the same here or there?

else BlankError
when Error then value
else Error.new # Blank error
end
end

Expand Down Expand Up @@ -75,7 +80,7 @@ def backtrace_for(ex, backtrace)
if trace[1]
# Ident stack trace for caller lines, to separate
# them from the main error lines.
trace[1..-1].each do |line|
trace[1..-1]&.each do |line|
backtrace << "\n\tfrom "
backtrace << line
end
Expand All @@ -92,9 +97,6 @@ def initialize(type = nil, message = nil, backtrace = nil)
@message = Utils.utf8_encode(message)
@backtrace = Utils.utf8_encode(backtrace)
end

BlankError = Error.new
ContainsMessage = ->(v) { v.respond_to?(:message) }
end
end
end
6 changes: 6 additions & 0 deletions sig/datadog.rbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module Datadog
include Core::Configuration
include Core::Extensions
include Tracing::Contrib::Extensions::Configuration
include Tracing::Contrib::Extensions::Helpers
end
31 changes: 31 additions & 0 deletions sig/datadog/core/buffer/random.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,37 @@ module Datadog
module Core
module Buffer
class Random
@closed: bool
@items: Array[Object]
Copy link
Member

Choose a reason for hiding this comment

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

Does it makes sense to extract it as a type alias here and use in all the latter sigs in this file?

@max_size: Integer

def initialize: (Integer)-> void
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def initialize: (Integer)-> void
def initialize: (Integer) -> void


def add!: (Object) -> Object

def add_all!: (Array[Object])-> void
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def add_all!: (Array[Object])-> void
def add_all!: (Array[Object]) -> void


def close: -> void

def closed?: -> bool

def concat: (Array[Object]) -> void

def drain!: -> Array[Object]

def empty?: -> bool

def full?: -> bool

def length: -> Integer

def overflow_segments: (Array[Object]) -> [Array[Object]?, Array[Object]?]

def pop: -> Array[Object]

def push: (Object) -> Object?

def replace!: (Object)-> Object?
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def replace!: (Object)-> Object?
def replace!: (Object) -> Object?

end
end
end
Expand Down
2 changes: 2 additions & 0 deletions sig/datadog/core/configuration.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ module Datadog
def logger: () -> Datadog::Core::Logger

def configuration: () -> untyped # The DSL methods have to be explicitly declared for this to be typed

def shutdown!: -> void
end
end
end
23 changes: 12 additions & 11 deletions sig/datadog/core/error.rbs
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
module Datadog
module Core
class Error
attr_reader type: untyped
attr_reader type: String

attr_reader message: untyped
attr_reader message: String

attr_reader backtrace: untyped
attr_reader backtrace: String

def self.build_from: (untyped value) -> untyped
interface _ContainsMessage
def message: () -> String
def class: () -> Class
end

def self.build_from: ((Error | [Object] | [Object, Object] | [Object, Object, Object] | ::Exception | _ContainsMessage | ::String) value) -> Error

private
def self.full_backtrace: (untyped ex) -> untyped
def self.backtrace_for: (untyped ex, untyped backtrace) -> (nil | untyped)
def self.full_backtrace: (Exception ex) -> String
def self.backtrace_for: (Exception ex, String backtrace) -> void

public

def initialize: (?untyped? `type`, ?untyped? message, ?untyped? backtrace) -> void

BlankError: untyped

ContainsMessage: untyped
def initialize: (?Object? `type`, ?Object? message, ?Object? backtrace) -> void
end
end
end
Loading