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

Fix Import collisions (take default imports into an account) #859

Merged
merged 5 commits into from May 8, 2023

Conversation

elb3k
Copy link
Contributor

@elb3k elb3k commented May 1, 2023

Name collision fixes do not take into account default imports here.

Sample input:

sort/sort.proto:

syntax = "proto3";
option go_package = "com.sort;sort";
package com.sort;

enum Enum {
  ENUM = 0;
}

sample.proto:

syntax = "proto3";

option go_package = "com.sample";
package com.sample;

import "validate/validate.proto";
import "sort/sort.proto";

message Message{
    sort.Enum enum = 1;
}

Output:

sample.pb.validate.go:

...

// ensure the imports are used
var (
	_ = bytes.MinRead
	_ = errors.New("")
	_ = fmt.Print
	_ = utf8.UTFMax
	_ = (*regexp.Regexp)(nil)
	_ = (*strings.Reader)(nil)
	_ = net.IPv4len
	_ = time.Duration(0)
	_ = (*url.URL)(nil)
	_ = (*mail.Address)(nil)
	_ = anypb.Any{}
	_ = sort.Sort

	_ = sort.Enum(0)
)
...

which should have been:

...

// ensure the imports are used
var (
	_ = bytes.MinRead
	_ = errors.New("")
	_ = fmt.Print
	_ = utf8.UTFMax
	_ = (*regexp.Regexp)(nil)
	_ = (*strings.Reader)(nil)
	_ = net.IPv4len
	_ = time.Duration(0)
	_ = (*url.URL)(nil)
	_ = (*mail.Address)(nil)
	_ = anypb.Any{}
	_ = sort.Sort

	_ = sort1.Enum(0)
)
...

@CLAassistant
Copy link

CLAassistant commented May 1, 2023

CLA assistant check
All committers have signed the CLA.

@elb3k elb3k marked this pull request as ready for review May 2, 2023 05:35
@elliotmjackson
Copy link
Contributor

Thanks for another great addition @elb3k! Can you add a test case to the harness so that we can ensure this doesn't regress?

@elb3k
Copy link
Contributor Author

elb3k commented May 3, 2023

Thanks for another great addition @elb3k! Can you add a test case to the harness so that we can ensure this doesn't regress?

Done. Not sure if this is the best way to test it.

@elliotmjackson elliotmjackson merged commit 4e25f91 into bufbuild:main May 8, 2023
5 checks passed
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.

None yet

3 participants