Skip to content
Draft
Show file tree
Hide file tree
Changes from 10 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
10 changes: 9 additions & 1 deletion spec/compiler/crystal/tools/init_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ private def run_init_project(skeleton_type, name, author, email, github_name, di
).run
end

private def yaml(value)
YAML::Any.new(value)
Comment thread
straight-shoota marked this conversation as resolved.
end

module Crystal
describe Init::InitProject do
it "has proper contents" do
Expand Down Expand Up @@ -146,7 +150,11 @@ module Crystal

with_file "example_app/shard.yml" do |shard_yml|
parsed = YAML.parse(shard_yml)
parsed["targets"].should eq({"example_app" => {"main" => "src/example_app.cr"}})
parsed["targets"].should eq({
yaml("example_app") => {
yaml("main") => "src/example_app.cr",
},
Comment on lines +179 to +181

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is not very intuitive. What prevents us from not modifying equality at all in this PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That is, I'd like this change to be gone but still have it compiling and working.

})
end

with_file "example/.travis.yml" do |travis|
Expand Down
4 changes: 4 additions & 0 deletions spec/std/hash_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ describe "Hash" do
a.should_not eq(c)
c.should_not eq(a)
d.should_not eq(a)

# Checking unions
Hash(String | Int32, Nil){32 => nil}.should eq Hash(Int32, Nil){32 => nil}
Hash(String, Nil){"hi" => nil}.should eq Hash(String | Int32, Nil){"hi" => nil}
end

it do
Expand Down
4 changes: 2 additions & 2 deletions spec/std/yaml/any_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ describe YAML::Any do
end

it "gets hash" do
YAML.parse("foo: bar").as_h.should eq({"foo" => "bar"})
YAML.parse("foo: bar").as_h?.should eq({"foo" => "bar"})
YAML.parse("foo: bar").as_h.should eq({YAML::Any.new("foo") => "bar"})
YAML.parse("foo: bar").as_h?.should eq({YAML::Any.new("foo") => "bar"})
YAML.parse("foo: bar")["foo"].as_h?.should be_nil
end

Expand Down
12 changes: 8 additions & 4 deletions spec/std/yaml/schema/core_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ private def it_parses_scalar_from_pull(string, file = __FILE__, line = __LINE__,
end
end

private def yaml(value) : YAML::Any
YAML::Any.new(value)
end

describe YAML::Schema::Core do
# nil
it_parses_scalar "~", nil
Expand Down Expand Up @@ -163,20 +167,20 @@ describe YAML::Schema::Core do
it_parses_scalar "N", "N"

# !!map
it_parses "!!map {1: 2}", {1 => 2}
it_parses "!!map {1: 2}", {yaml(1i64) => 2}
it_raises_on_parse "!!map 1", "Expected MAPPING_START"

# !!omap
it_parses "!!omap {1: 2}", {1 => 2}
it_parses "!!omap {1: 2}", {yaml(1i64) => 2}
it_raises_on_parse "!!omap 1", "Expected MAPPING_START"

# !!pairs
it_parses "!!pairs [{1: 2}, {3: 4}]", [{1 => 2}, {3 => 4}]
it_parses "!!pairs [{1: 2}, {3: 4}]", [{yaml(1i64) => 2}, {yaml(3i64) => 4}]
it_raises_on_parse "!!pairs 1", "Expected SEQUENCE_START"
it_raises_on_parse "!!pairs [{1: 2, 3: 4}]", "Expected MAPPING_END"

# !!set
it_parses "!!set { 1, 2, 3 }", Set{1, 2, 3}
it_parses "!!set { 1, 2, 3 }", Set{yaml(1i64), yaml(2i64), yaml(3i64)}
it_raises_on_parse "!!set 1", "Expected MAPPING_START"

# !!seq
Expand Down
19 changes: 15 additions & 4 deletions spec/std/yaml/schema/fail_safe_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,31 @@ private def it_raises_on_parse_all(string, message, file = __FILE__, line = __LI
end
end

private def yaml(value)
YAML::Any.new(value)
end

describe YAML::Schema::FailSafe do
# parse
it_parses "123", "123"
it_parses %(
context:
replace_me: "Yes please!"
), {"context" => {"replace_me" => "Yes please!"}}
), {yaml("context") => {yaml("replace_me") => "Yes please!"}}
it_parses %(
first:
document:

second:
document:
), {"first" => {"document" => ""}, "second" => {"document" => ""}}
), {
yaml("first") => {
yaml("document") => "",
},
yaml("second") => {
yaml("document") => "",
},
}
it_raises_on_parse %(
this: "gives"
an: "error"
Expand All @@ -54,14 +65,14 @@ describe YAML::Schema::FailSafe do
it_parses_all %(
context:
replace_me: "Yes please!"
), [{"context" => {"replace_me" => "Yes please!"}}]
), [{yaml("context") => {yaml("replace_me") => "Yes please!"}}]
it_parses_all %(
foo:
bar: 123

bar:
foo: 321
), [{"foo" => {"bar" => "123"}, "bar" => {"foo" => "321"}}]
), [{yaml("foo") => {yaml("bar") => "123"}, yaml("bar") => {yaml("foo") => "321"}}]
it_raises_on_parse_all %(
this: "raises"
an: "yaml"
Expand Down
25 changes: 19 additions & 6 deletions spec/std/yaml/yaml_spec.cr
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
require "spec"
require "yaml"

private def yaml(value)
YAML::Any.new(value)
end

describe "YAML" do
describe "parser" do
it { YAML.parse("foo").should eq("foo") }
it { YAML.parse("- foo\n- bar").should eq(["foo", "bar"]) }
it { YAML.parse_all("---\nfoo\n---\nbar\n").should eq(["foo", "bar"]) }
it { YAML.parse("foo: bar").should eq({"foo" => "bar"}) }
it { YAML.parse("foo: bar").should eq({yaml("foo") => "bar"}) }
it { YAML.parse("--- []\n").should eq([] of YAML::Any) }
it { YAML.parse("---\n...").should eq nil }

Expand Down Expand Up @@ -56,7 +60,7 @@ describe "YAML" do
bar:
<<: *x
))
doc["bar"].should eq({"bar" => 1, "baz" => 2})
doc["bar"].should eq({yaml("bar") => 1, yaml("baz") => 2})
end

it "merges other mapping with array of alias" do
Expand All @@ -68,7 +72,7 @@ describe "YAML" do
bar:
<<: [*x, *y]
))
doc["bar"].should eq({"bar" => 1, "baz" => 2})
doc["bar"].should eq({yaml("bar") => 1, yaml("baz") => 2})
end

it "doesn't merge explicit string key <<" do
Expand All @@ -78,7 +82,16 @@ describe "YAML" do
bar:
!!str '<<': *foo
))
doc.should eq({"foo" => {"hello" => "world"}, "bar" => {"<<" => {"hello" => "world"}}})
doc.should eq({
yaml("foo") => {
yaml("hello") => "world",
},
yaml("bar") => {
yaml("<<") => {
yaml("hello") => "world",
},
},
})
end

it "doesn't merge empty mapping" do
Expand All @@ -87,7 +100,7 @@ describe "YAML" do
bar:
<<: *foo
))
doc["bar"].should eq({"<<" => nil})
doc["bar"].should eq({yaml("<<") => nil})
end

it "doesn't merge arrays" do
Expand All @@ -97,7 +110,7 @@ describe "YAML" do
bar:
<<: *foo
))
doc["bar"].should eq({"<<" => [1]})
doc["bar"].should eq({yaml("<<") => [1]})
end

it "has correct line/number info (#2585)" do
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/crystal/program.cr
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,11 @@ module Crystal
file_module?(filename).try &.lookup_matches(signature)
end

def file_module?(filename)
def file_module?(filename : String)
file_modules[filename]?
end

def file_module(filename)
def file_module(filename : String)
file_modules[filename] ||= FileModule.new(self, self, filename)
end

Expand Down
8 changes: 4 additions & 4 deletions src/compiler/crystal/semantic/type_declaration_processor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ struct Crystal::TypeDeclarationProcessor
ancestor = owner.ancestors.first?
ancestor = uninstantiate(ancestor)
if ancestor
ancestor_non_nilable = @non_nilable_instance_vars[ancestor]?
ancestor_non_nilable = @non_nilable_instance_vars[ancestor]? if ancestor.is_a?(Type)
Comment thread
jgaskins marked this conversation as resolved.
Outdated
end

# If the ancestor has non-nilable instance vars, check that all initialize either call
Expand Down Expand Up @@ -538,7 +538,7 @@ struct Crystal::TypeDeclarationProcessor

owner.ancestors.any? do |ancestor|
ancestor = uninstantiate(ancestor)
@instance_vars_outside[ancestor]?.try &.includes?(name)
@instance_vars_outside[ancestor]?.try &.includes?(name) if ancestor.is_a? Type
end
end

Expand All @@ -553,7 +553,7 @@ struct Crystal::TypeDeclarationProcessor
end

private def compute_non_nilable_outside_single(owner, non_nilable_outisde)
if vars = @instance_vars_outside[owner]?
if owner.is_a?(Type) && (vars = @instance_vars_outside[owner]?)
non_nilable_outisde ||= [] of String
vars.each do |name|
non_nilable_outisde << name unless non_nilable_outisde.includes?(name)
Expand All @@ -570,7 +570,7 @@ struct Crystal::TypeDeclarationProcessor

owner.ancestors.each do |ancestor|
ancestor = uninstantiate(ancestor)
infos = @initialize_infos[ancestor]?
infos = @initialize_infos[ancestor]? if ancestor.is_a?(Type)
return infos if infos && !infos.empty?
end

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/tools/playground/server.cr
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ module Crystal::Playground

agent_ws = PathWebSocketHandler.new "/agent" do |ws, context|
match_data = context.request.path.not_nil!.match(/\/(\d+)\/(\d+)$/).not_nil!
session_key = match_data[1]?.try(&.to_i)
session_key = match_data[1]?.try(&.to_i) || 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

session_key can't be nil because of this line. Because it can't store nil and it's using [] instead of []? below, this line seems like it can't return nil even without this PR or it would raise a KeyError below.

tag = match_data[2]?.try(&.to_i)
@logger.info "#{context.request.path} WebSocket connected (session=#{session_key}, tag=#{tag})"

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/tools/typed_def_processor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module Crystal::TypedDefProcessor

private def process_result(result : Compiler::Result)
process_type result.program
if file_module = result.program.file_module? target_location.original_filename
if (filename = target_location.original_filename) && (file_module = result.program.file_module?(filename))
process_type file_module
end
end
Expand Down
Loading