أرغب في مشاركة نتائج الأيام القليلة الماضية ، والتي شملت تحليل بوابة تاريخ بعض مشاريع Go الكبيرة بهدف إيجاد تعهدات بتصحيح الأخطاء ثم إضفاء الطابع الرسمي عليها للكشف عنها إذا ظهرت في التعليمات البرمجية الجديدة.

في الجزء الثاني من المقالة ، سننظر في بعض التشخيصات الجديدة في برنامج go-الناقد ، والذي يسمح لك بالعثور على رمز يحتمل أن يحتوي على خطأ.

البحث عن الأفكار في الماضي

للعثور على أفكار لعمليات فحص التعليمات البرمجية الجديدة ، يمكنك إلقاء نظرة خاطفة على أجهزة التحليل الثابتة للغات البرمجة الأخرى ، أو يمكنك فحص التعليمات البرمجية المصدر المفتوح يدويًا في محاولة للعثور على قوالب التعليمات البرمجية الخاطئة ، أو يمكنك البحث في السجل.

افترض أنك تقوم بفحص مشروع Go-Files . بعد البدء في linter (محلل ثابت) ، لا توجد مشاكل ؛ لم تحقق مراجعة نصوص المصدر أيضًا نتائج رائعة. هل يجب علي الاندفاع لوضع Go-Files جانباً؟ ليس حقا

أثناء وجودها ، كان لدى Go-Files عدد من العيوب التي يمكن اكتشافها بسهولة ، ولكن الآن تم إصلاحها بالفعل. ما يتعين علينا القيام به هو البدء في تحليل تاريخ مستودع الجيت.

البق في مكان ما في مكان قريب.

مجموعة من الالتزامات التي تهمنا

المشاكل:

• يمكن أن يكون هناك الكثير من الالتزامات
• في بعض الأحيان في نفس المراجعة لا علاقة لبعضها البعض
• في بعض الأحيان ، لا يتطابق تعليق على التزام مع التغييرات الفعلية تمامًا.
• البعض يقلل من استخدام رسالة الالتزام الجيدة (دعونا لا نشير الأصابع)

بادئ ذي بدء ، نريد تقليل كمية المعلومات التي يجب معالجتها دون التشغيل الآلي. يمكن حل مشكلة الاستجابات الخاطئة السلبية عن طريق زيادة حجم العينة (قم بتنزيل المزيد من حزم Go إلى مجموعة التحكم).

يمكنك تحليله بطرق مختلفة ، لقد تلقيت للتو git log --grep لمجموعة من القوالب ، مع المبالغة في تقدير والالتزام بنتيجة الالتزام وفقًا لمحتويات رسالة الالتزام. إذا كان الالتزام كبيرًا جدًا ، فيمكنك أن تتجاهله على الفور ، لأنه من المحتمل جدًا معرفة ما سيحدث أنه لن يكون هناك أي منافس.

أود أيضًا أن أضيف أنه إلى جانب التصحيحات أو العثور على أخطاء في كثير من الأحيان لا يوجد وصف مفرط في التفاؤل وأحيانًا ليس ثقافيًا تمامًا:

 - "We check length later, but we assumed it was always 1 bytes long. Not always the case. I'm a little depressed that this bug was there" - "Really fixed the bug now" - "WTF, the changes that actually fixed the bug for the accesspattern wasn't actually committed" - "how do i pronounce this damn project" ( traefic) - "damnit travis" - "one f*cking dot" (./.. => ./...  .travis.yml) 

معظم "إصلاحات الأخطاء" عديمة الجدوى هي تحرير أخطاء بناء الجملة أو غيرها من المشاكل التي تمنع المشروع من التحويل البرمجي حتى من خلال go build . لا تستخدم جميع المشروعات CI أو ربط مسبق ، لذلك ينتهي الأمر بهذه المراجعات المعطلة في بعض الأحيان في الفرع الرئيسي.

أخطاء تاريخية

دعنا نذهب من خلال بعض الأخطاء الأكثر إثارة للاهتمام التي تم العثور عليها تحت أغلفة سجلات بوابة.

عودة لا شيء بدلا من النتيجة الحقيقية

gin-gonic / gin: إصلاح الخلل ، إرجاع الخطأ عندما يفشل ربط منطقي :

  if err == nil { field.SetBool(boolVal) } - return nil + return err } 

go-pg / pg: إصلاح أخطاء AppendParam :

  return method.AppendValue(b, m.strct.Addr(), 1), true } - return nil, false + return b, false } 

عدم العودة في مسار سريع

gonum / gonum: خطأ ثابت في المسار السريع dswap :

  for i := 0; i < n; i++ { x[i], y[i] = y[i], x[i] } + return } 

تشغيل goroutines دون wg.Add (1)

btcsuite / btcd: إصلاح العديد من الأخطاء في إيقاف تشغيل خادم RPC :

 +s.wg.Add(1) go s.walletListenerDuplicator() 

حالة منطقية غير صالحة

gorgonia / gorgonia: تم إصلاح بعض الأخطاء أيضًا :

 -for i := 0; i > start; i++ { +for i := 0; i < start; i++ { retVal[i] = 0 } 

src-d / go-git: السباكة / idxfile: إصلاح أخطاء البحث في MemoryIndex :

 -if low < high { +if low > high { break } 

go-xorm / xorm: إصلاح الخلل في المجموع :

 -if !strings.Contains(colName, " ") && strings.Contains(colName, "(") { +if !strings.Contains(colName, " ") && !strings.Contains(colName, "(") { 

btcsuite / btcd: server: إصلاح الخلل ، فصل نظير على filteradd :

 -if sp.filter.IsLoaded() { +if !sp.filter.IsLoaded() { 

btcsuite / btcd: إصلاح خطأ الاختبار العكسي باستخدام الحد الأقصى لعمليات التشغيل :

 -if pop.opcode.value < OP_16 { +if pop.opcode.value > OP_16 { s.numOps++ 

تحديث المتلقي (هذا / الذات) مرت بالقيمة

الكلاسيكية يتم تعديل نسخة من الكائن داخل الطريقة ، والتي بسببها لن يرى المتصل النتائج المتوقعة.

إصلاح الخلل المكدس / قائمة الانتظار / deque (مستقبل المؤشر) :

 -func (s Stack) Push(x interface{}) { - s = append(s, x) +func (s *Stack) Push(x interface{}) { + *s = append(*s, x) } 

تحديث نسخ من الكائنات داخل حلقة

containous / traefik: عيّن المهام التي تمت تصفيتها إلى التطبيقات الموجودة في الشريحة :

 -for _, app := range filteredApps { - app.Tasks = fun.Filter(func(task *marathon.Task) bool { +for i, app := range filteredApps { + filteredApps[i].Tasks = fun.Filter(func(task *marathon.Task) bool { return p.taskFilter(*task, app) }, app.Tasks).([]*marathon.Task) 

يحتوي على / traefik: إضافة اختبارات وحدة لخزينة آمنة :

 -for _, routine := range p.routines { +for i := range p.routines { p.waitGroup.Add(1) - routine.stop = make(chan bool, 1) + p.routines[i].stop = make(chan bool, 1) Go(func() { - routine.goroutine(routine.stop) + p.routines[i].goroutine(p.routines[i].stop) p.waitGroup.Done() }) } 

لا يتم استخدام نتيجة وظيفة الالتفاف.

gorgonia / gorgonia: تم إصلاح خطأ صغير غير تالٍ في Grad () :

 if !n.isInput() { - errors.Wrapf(err, ...) - // return + err = errors.Wrapf(err, ...) + return nil, err } 

العمل غير الصحيح مع جعل

في بعض الأحيان ، يقوم "الشباب" بعمل " make([]T, count) " make([]T, count) يليه append داخل الحلقة. الخيار الصحيح هنا هو " make([]T, 0, count) ".

HouzuoGuo / tiedot: إصلاح خطأ مسح مجموعة :

 -ids := make([]uint64, benchSize) +ids := make([]uint64, 0) 

يقوم الإصلاح أعلاه بتصحيح الخطأ الأصلي ، لكن به عيب واحد تم إصلاحه في أحد الإخطارات التالية:

 -ids := make([]uint64, 0) +ids := make([]uint64, 0, benchSize) 

لكن بعض العمليات ، مثل copy أو ScanSlice قد تتطلب الطول "الصحيح".

go-xorm / xorm: إصلاح الخلل في SumsInt لإرجاع شريحة فارغة :

 -var res = make([]int64, 0, len(columnNames)) +var res = make([]int64, len(columnNames), len(columnNames)) if session.IsAutoCommit { err = session.DB().QueryRow(sqlStr, args...).ScanSlice(&res) } else { 

قريبًا ، سيساعد الناقدون في العثور على أخطاء في هذه الفئة.

أخطاء أخرى

تبين أن التعداد مرهق إلى حد ما ، فأنا أخرج الباقي تحت المفسد. هذا الجزء اختياري ، لذا يمكنك تركه للمضي قدمًا أو بجرأة.

 -for _, el := range *s1 { +for el := range *s1 { if _, ok := (*s2)[el]; !ok { return false } 

 func (s *server) Query(ctx context.Context, - req *graph.Request) (resp *graph.Response, err error) { + req *graph.Request) (*graph.Response, error) { + resp := new(graph.Response) 

 -block, err := btcutil.NewBlockFromBytes(blockBytes) +block, err = btcutil.NewBlockFromBytes(blockBytes) if err != nil { return err } 

 -err = session.Rollback() +err1 := session.Rollback() +if err1 == nil { + return lastId, err +} +err = err1 

 -debugPrint("Listening and serving HTTP on %s", addr) +debugPrint("Listening and serving HTTP on %s\n", addr) 

  styleCache map[int]*Style `-` + lock *sync.RWMutex } 

حملة صليبية ضد الكود المشبوه

badCond

badCond عن تعبيرات منطقية غير صحيحة.

أمثلة على التعبيرات المنطقية المشبوهة:

• والنتيجة هي دائما true أو false
• التعبير مكتوب بشكل لا يمكن تمييزه تقريبًا عن الخاطئ

يعمل هذا الاختيار أيضًا على عمليات مثل " x == nil && x == y ". أحد الحلول البسيطة هو إعادة الكتابة إلى " x == nil && y == nil ". لا تعاني قابلية قراءة التعليمات البرمجية ، ولكن لن يرى linter أي شيء مشبوه في هذا الرمز.

فيما يلي مثال لإصلاح الأخطاء التي يمكن العثور عليها بواسطة badCond :

 -if err1 := f(); err != nil && err == nil { +if err1 := f(); err1 != nil && err == nil { err = err1 } 

الجوائز:

• moby / buildkit: لقطة: إصلاح حالة مستحيلة
• tdewolff / تحليل: EqualFold من util.go لديه حالة غريبة
• HouzuoGuo / tiedot: httpapi: حالة مشبوهة في srv_test.go

الضعيف

weakCond يشبه badCond ، لكن مستوى تنبيهات الثقة أقل قليلاً.

الشرط الضعيف هو الشرط الذي لا يغطي الإدخال بالكامل بما فيه الكفاية.

من الأمثلة الجيدة لحالة ضعيفة (غير كافية) التحقق من الشريحة " nil ثم استخدام العنصر الأول. الشروط " s != nil " ليست كافية هنا. الشرط الصحيح هو " len(s) != 0 " أو ما يعادلها:

 func addRequiredHeadersToRedirectedRequests( req *http.Request, via []*http.Request) error { - if via != nil && via[0] != nil { + if len(via) != 0 && via[0] != nil { 

الجوائز:

• etcd-io / etcd: etcdserver / api / v2v3: الفهرسة بعد التحقق من الصفر دون التحقق من اللين
• moby / moby: التسجيل: فهرسة بعد التحقق من عدم الاختيار دون
• inkyblackness / hacked: ss1 / content / text: فهرسة بعد التحقق من عدم الاختيار دون len

dupArg

بالنسبة لبعض الوظائف ، لا يكون لإعطاء نفس القيمة (أو المتغير) مثل الوسائط المتعددة معنى كبير. في كثير من الأحيان ، وهذا يشير إلى خطأ نسخ / لصق .

مثال على هذه الوظيفة هو copy . copy(xs, xs) التعبير copy(xs, xs) لا معنى لها.

 -if !bytes.Equal(i2.Value, i2.Value) { +if !bytes.Equal(i1.Value, i2.Value) { return fmt.Errorf("tries differ at key %x", i1.Key) } 

الجوائز:

• ethereum / go-ethereum: light: fix وسيطة مكررة في استدعاء بايت
• openhift / الأصل: إصلاح reflect.DeepEqual (س ، س) علة

dupCase

ربما تعلم أنه في Go لا يمكنك استخدام قيم case المكررة داخل switch .

المثال أدناه لا يتم ترجمة :

 switch x := 0; x { case 1: fmt.Println("first") case 1: fmt.Println("second") } 

خطأ في التجميع: الحالة المكررة 1 في التبديل.

ولكن ماذا لو أخذنا مثالاً أكثر إثارة للاهتمام :

 one := 1 switch x := 1; x { case one: fmt.Println("first") case one: fmt.Println("second") } 

يتم السماح بالقيم المكررة من خلال المتغير. بالإضافة إلى ذلك ، يعطي switch true مساحة أكبر للأخطاء:

 switch x := 0; true { case x == 1: fmt.Println("first") case x == 1: fmt.Println("second") } 

وإليك مثال لإصلاح خطأ حقيقي:

 switch { case m < n: // Upper triangular matrix. //   . case m == n: // Diagonal matrix. //   . -case m < n: // Lower triangular matrix. +case m > n: // Lower triangular matrix. //   . } 

الجوائز:

• mellium / sasl: إزالة حالة التبديل المكررة
• gonum / gonum: إصلاح حالة التبديل مكررة
• حلف الأساس / pact-go: إزالة حالة التبديل المكررة
• minio / minio: بند حالة التبديل المزدوج المشبوه في cmd / object-api-error.go
• henrylee2cn / pholcus: common / bytes: إزالة حالة التبديل المكررة
• go-ble / ble: linux / att: إصلاح خطأ حالة التبديل المكررة
• عامل ميناء / التوزيع: التسجيل / التخزين: إصلاح حالة التبديل مكررة

dupBranchBody

البيانات الشرطية مثل if/else و switch لها هيئات للتنفيذ. عند استيفاء الشرط ، يتم نقل التحكم إلى وحدة العمليات المرتبطة. في حين أن التشخيصات الأخرى تتحقق من اختلاف شروط هذه الكتل ، فإن dupBranchBody تتحقق من أن الكتل القابلة للتنفيذ نفسها ليست متطابقة تمامًا.

وجود هيئات مكررة داخل البيانات الشرطية ، ما لم يكن type switch ، أمر مشبوه على الأقل.

 -if s, err := r.ReceivePack(context.Background(), req); err != nil { - return s, err -} else { - return s, err -} +return r.ReceivePack(context.Background(), req) 

يتم ترك الأحكام المتعلقة بدرجة صحة الشفرة أدناه للقارئ:

 if field.IsMessage() || p.IsGroup(field) { //   . } else if field.IsString() { if nullable && !proto3 { p.generateNullableField(fieldname, verbose) } else { pP(`if this.`, fieldname, ` != that1.`, fieldname, `{`) } } else { if nullable && !proto3 { p.generateNullableField(fieldname, verbose) } else { pP(`if this.`, fieldname, ` != that1.`, fieldname, `{`) } } } 

الهيئات داخل " if field.IsString() " و " else " المرتبطة متطابقة.

الجوائز:

• src-d / go-git: الهيئات الفرعية المكررة المشبوهة

caseOrder

type switch فرز جميع الأنواع الموجودة داخل type switch بالتتابع ، لأول واحد متوافق. إذا كانت قيمة العلامة من النوع *T ، وتنفذ الواجهة I ، فأنت بحاجة إلى وضع علامة مع " case *T " قبل التسمية مع " case I " ، وإلا سيتم تنفيذ " case I " فقط ، لأن *T متوافق مع I (لكن ليس العكس.

 case string: res = append(res, NewTargetExpr(v).toExpr().(*expr)) -case Expr: - res = append(res, v.toExpr().(*expr)) case *expr: res = append(res, v) +case Expr: + res = append(res, v.toExpr().(*expr)) 

الجوائز:

• كمد / المعلم: إصلاح ترتيب حالة غير صحيح في description.go
• رسالة / خط أنابيب: إصلاح نوع تبديل حالة النظام
• coyove / goflyway: الوكيل: إصلاح ترتيب الحالة في تبديل نوع
• go-graphite / carbonapi: pkg / parser: fix order switch case order

offBy1

خطأ لكل وحدة شائع جدًا لدرجة أنه يحتوي على صفحة ويكيبيديا الخاصة بها .

 if len(optArgs) > 1 { return nil, ErrTooManyOptArgs } -node = optArgs[1] +node = optArgs[0] 

 -last := xs[len(xs)] +last := xs[len(xs) - 1] 

لدى go-critic خيارات محدودة للعثور على مثل هذه الأخطاء ، ولكن حتى الآن لم يتم إرسال إصلاح واحد إلى المستودع العام. فيما يلي بعض الإصلاحات التي تم العثور عليها أثناء عرض القصة:

• btcsuite / btcd: اختبار وإصلاح الأخطاء في getaddednodeinfo
• btcsuite / btcd: تحسين تغطية الاختبار وإصلاح بعض الأخطاء التي عثر عليها في الاختبارات

قليلا من الرعب في النهاية

لدى go vet فحصًا جيدًا لعبارات مثل " x != a || x != b ". يبدو أن الناس قد لا يعرفون عن gometalinter ، ولكن go vet تطلق الجميع تقريبا ، أليس كذلك؟

باستخدام أداة gogrep ، وضعت قائمة صغيرة من التعبيرات المماثلة التي لا تزال في الفروع الرئيسية لبعض المشاريع.

 cloud.google.com/go/trace/trace_test.go:943:7: expectTraceOption != ((o2&1) != 0) || expectTraceOption != ((o3&1) != 0) github.com/Shopify/sarama/mocks/sync_producer_test.go:36:5: offset != 1 || offset != msg.Offset github.com/Shopify/sarama/mocks/sync_producer_test.go:44:5: offset != 2 || offset != msg.Offset github.com/docker/libnetwork/api/api_test.go:376:5: id1 != i2e(i2).ID || id1 != i2e(i3).ID github.com/docker/libnetwork/api/api_test.go:408:5: "sh" != epList[0].Network || "sh" != epList[1].Network github.com/docker/libnetwork/api/api_test.go:1196:5: ep0.ID() != ep1.ID() || ep0.ID() != ep2.ID() github.com/docker/libnetwork/api/api_test.go:1467:5: ep0.ID() != ep1.ID() || ep0.ID() != ep2.ID() github.com/docker/libnetwork/ipam/allocator_test.go:1261:5: len(indices) != len(allocated) || len(indices) != num github.com/esimov/caire/grayscale_test.go:27:7: r != g || r != b github.com/gogo/protobuf/test/bug_test.go:99:5: protoSize != mSize || protoSize != lenData github.com/gogo/protobuf/test/combos/both/bug_test.go:99:5: protoSize != mSize || protoSize != lenData github.com/gogo/protobuf/test/combos/marshaler/bug_test.go:99:5: protoSize != mSize || protoSize != lenData github.com/gogo/protobuf/test/combos/unmarshaler/bug_test.go:99:5: protoSize != mSize || protoSize != lenData github.com/gonum/floats/floats.go:65:5: len(dst) != len(s) || len(dst) != len(y) github.com/gonum/lapack/testlapack/dhseqr.go:139:7: wr[i] != h.Data[i*h.Stride+i] || wr[i] != h.Data[(i+1)*h.Stride+i+1] github.com/gonum/stat/stat.go:1053:5: len(x) != len(labels) || len(x) != len(weights) github.com/hashicorp/go-sockaddr/ipv4addr_test.go:659:27: sockaddr.IPPort(p) != test.z16_portInt || sockaddr.IPPort(p) != test.z16_portInt github.com/hashicorp/go-sockaddr/ipv6addr_test.go:430:27: sockaddr.IPPort(p) != test.z16_portInt || sockaddr.IPPort(p) != test.z16_portInt github.com/nats-io/gnatsd/server/monitor_test.go:1863:6: v.ID != c.ID || v.ID != r.ID github.com/nbutton23/zxcvbn-go/adjacency/adjcmartix.go:85:7: char != "" || char != " " github.com/openshift/origin/pkg/oc/cli/admin/migrate/migrator_test.go:85:7: expectedInfos != writes || expectedInfos != saves github.com/openshift/origin/test/integration/project_request_test.go:120:62: added != deleted || added != 1 github.com/openshift/origin/test/integration/project_request_test.go:126:64: added != deleted || added != 4 github.com/openshift/origin/test/integration/project_request_test.go:132:62: added != deleted || added != 1 gonum.org/v1/gonum/floats/floats.go:60:5: len(dst) != len(s) || len(dst) != len(y) gonum.org/v1/gonum/lapack/testlapack/dhseqr.go:139:7: wr[i] != h.Data[i*h.Stride+i] || wr[i] != h.Data[(i+1)*h.Stride+i+1] gonum.org/v1/gonum/stat/stat.go:1146:5: len(x) != len(labels) || len(x) != len(weights) 

التعلم من أخطاء الآخرين

لا ، حتى في الجزء الأخير لن يكون هناك شيء عن التعلم الآلي .

ولكن ما سيتم كتابته هنا هو حول golangci-lint ، والذي قام بدمج الناقد في واحدة من أحدث الإصدارات. golangci-lint عبارة عن تناظرية لل gometalinter ، والتي يمكنك قراءتها حول مزايا ذلك ، على سبيل المثال ، في مقال "التحليل الثابت في Go: كيف يمكننا توفير الوقت عند التحقق من الكود" .

تم إعادة كتابة Go- go-critic مؤخرًا باستخدام lintpack . يمكن العثور على التفاصيل في Go lintpack: مدير linter composable .

إذا لم تكن قد بدأت بعد في استخدام الأدوات بنشاط لتحليل برامجك بحثًا عن الأخطاء المحتملة ، سواء الأسلوبية أو المنطقية ، فإنني أوصي اليوم مرة أخرى بالتفكير في سلوكك على فنجان من الشاي مع الزملاء. سوف تنجح.

جيد للجميع.